-
-
Notifications
You must be signed in to change notification settings - Fork 2.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
WIP Initial version of updated code for R transpiler #449
Changes from 3 commits
e39bcf1
23bb710
9ce9509
ed5d5db
3df1cd6
451283f
5998545
6606943
4575b9f
9bb7603
f1e47f5
f4cd844
aae11d6
62bf001
89b7b85
664e516
3bf21b3
84a6745
2a4ac5a
3341d8e
0d7943d
e182736
13d0d71
8a43d98
8d2d538
008598f
918a073
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -387,6 +387,120 @@ def __repr__(self): | |
required_args = required_props(props) | ||
return c.format(**locals()) | ||
|
||
def generate_class_string_r(typename, props, description, namespace): | ||
""" | ||
Dynamically generate class strings to have nicely formatted docstrings, | ||
keyword arguments, and repr | ||
|
||
Inspired by http://jameso.be/2013/08/06/namedtuple.html | ||
|
||
Parameters | ||
---------- | ||
typename | ||
props | ||
description | ||
namespace | ||
|
||
Returns | ||
------- | ||
string | ||
|
||
""" | ||
# TODO _prop_names, _type, _namespace, available_events, | ||
# and available_properties | ||
# can be modified by a Dash JS developer via setattr | ||
# TODO - Tab out the repr for the repr of these components to make it | ||
# look more like a hierarchical tree | ||
# TODO - Include "description" "defaultValue" in the repr and docstring | ||
# | ||
# TODO - Handle "required" | ||
# | ||
# TODO - How to handle user-given `null` values? I want to include | ||
# an expanded docstring like Dropdown(value=None, id=None) | ||
# but by templating in those None values, I have no way of knowing | ||
# whether a property is None because the user explicitly wanted | ||
# it to be `null` or whether that was just the default value. | ||
# The solution might be to deal with default values better although | ||
# not all component authors will supply those. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Remove those comments, they contains todo's that are specifics to python classes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will do. I'll also finish commenting my additions where appropriate. |
||
c = '''{docstring} | ||
|
||
html{typename} <- function(..., {default_argtext}) {{ | ||
|
||
component <- list( | ||
props = list( | ||
{default_paramtext} | ||
), | ||
type = '{typename}', | ||
namespace = '{namespace}', | ||
propNames = c({list_of_valid_keys}), | ||
package = '{package_name}' | ||
) | ||
|
||
component$props <- filter_null(component$props) | ||
component <- append_wildcard_props(component, wildcards = c({default_wildcards}), ...) | ||
|
||
structure(component, class = c('dash_component', 'list')) | ||
}} | ||
''' | ||
|
||
filtered_props = reorder_props(filter_props(props)) | ||
# pylint: disable=unused-variable | ||
list_of_valid_wildcard_attr_prefixes = repr(parse_wildcards(props)) | ||
# pylint: disable=unused-variable | ||
list_of_valid_keys = repr(list(map(str, filtered_props.keys()))) | ||
list_of_valid_keys = list_of_valid_keys[1:-1] | ||
package_name = make_package_name(namespace) | ||
# pylint: disable=unused-variable | ||
docstring = create_docstring_r( | ||
component_name=typename, | ||
props=filtered_props, | ||
events=parse_events(props), | ||
description=description).replace('\r\n', '\n') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added that There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually, it is a consideration in R as well, though I think I should remove the |
||
|
||
# pylint: disable=unused-variable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ |
||
# want to remove data-*, aria-* | ||
events = '[' + ', '.join(parse_events(props)) + ']' | ||
prop_keys = list(props.keys()) | ||
default_paramtext = '' | ||
default_wildcards = '' | ||
wildcard_list = '' | ||
for key in props: | ||
if '*' in key: | ||
wildcard_list.join(key) | ||
if 'children' in props: | ||
prop_keys.remove('children') | ||
default_argtext = "children=NULL, " | ||
# pylint: disable=unused-variable | ||
argtext = 'children=children, **args' | ||
else: | ||
default_argtext = "" | ||
argtext = '**args' | ||
# in R, we set parameters with no defaults to NULL, here we'll do that if no default value exists | ||
default_wildcards += ", ".join( | ||
[('\'{:s}\''.format(p)) | ||
for p in prop_keys | ||
if '*' in p] | ||
) | ||
default_argtext += ", ".join( | ||
[('{:s}={}'.format(p, props[p]['defaultValue']['value']) | ||
if 'defaultValue' in props[p] else | ||
'{:s}=NULL'.format(p)) | ||
for p in prop_keys | ||
if not p.endswith("-*") and | ||
p not in keyword.kwlist and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ This keyword list filter is for python keywords, won't be the same in R. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually now we manually maintain a file of python keywords (https://github.com/plotly/dash/blob/master/dash/development/_all_keywords.py). Perhaps we can re-name that to python_keywords and make a new one called R_keywords. That is, if this problem even exists in R. Can you define a function with argument names that are the same as a reserved keyword in R? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
If the function argument matches a reserved word (which is not first escaped with backticks), the R parser will throw an error:
The help page (also produced when you type https://stat.ethz.ch/R-manual/R-devel/library/base/html/Reserved.html I've gone ahead and produced a GitHub gist including reserved words in R, similar to the keyword list already available for Python. I can submit it as a pull request if it would be helpful. https://gist.github.com/rpkyle/622155e24f603fc54a664b06f8036f3d There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You can include those keywords in https://github.com/plotly/dash/blob/master/dash/development/_all_keywords.py as a new variable There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Great, will do. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm in the process of updating the code I've written, so please hold off on reviewing the updates to this PR for a day or so. Nicolas suggested remerging my new branch with R, so that we can keep the current PR alive. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||
p not in ['setProps']] | ||
) | ||
default_paramtext += ", ".join( | ||
[('{:s}={:s}'.format(p, p) | ||
if p != "children" else | ||
'{:s}=c(children, assert_valid_children(..., wildcards = c({:s})))'.format(p, default_wildcards)) | ||
for p in props.keys() | ||
if not p.endswith("-*") and | ||
p not in keyword.kwlist and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ |
||
p not in ['setProps']] | ||
) | ||
required_args = required_props(props) | ||
return c.format(**locals()) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like them to be explicitly stated instead of locals(), then we can remove all those There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, I think we talked about this when you explained list comprehensions to me a while ago. Will remove the shorthand and explicitly declare the variables. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with this, and we should do it in the Python version as well. I've added a note here: #311 |
||
|
||
# pylint: disable=unused-argument | ||
def generate_class_file(typename, props, description, namespace): | ||
|
@@ -421,6 +535,36 @@ def generate_class_file(typename, props, description, namespace): | |
f.write(import_string) | ||
f.write(class_string) | ||
|
||
# pylint: disable=unused-argument | ||
def generate_class_file_r(typename, props, description, namespace): | ||
""" | ||
Generate a R class file (.R) given a class string | ||
|
||
Parameters | ||
---------- | ||
typename | ||
props | ||
description | ||
namespace | ||
|
||
Returns | ||
------- | ||
|
||
""" | ||
import_string =\ | ||
"# AUTO GENERATED FILE - DO NOT EDIT\n\n" | ||
class_string = generate_class_string_r( | ||
typename, | ||
props, | ||
description, | ||
namespace | ||
) | ||
file_name = "{:s}.R".format(typename) | ||
|
||
file_path = os.path.join(namespace, file_name) | ||
with open(file_path, 'w') as f: | ||
f.write(import_string) | ||
f.write(class_string) | ||
|
||
# pylint: disable=unused-argument | ||
def generate_class(typename, props, description, namespace): | ||
|
@@ -445,6 +589,27 @@ def generate_class(typename, props, description, namespace): | |
result = scope[typename] | ||
return result | ||
|
||
def generate_class_r(typename, props, description, namespace): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ the generate_class for python is for dynamically loaded components, we don't do that in R. |
||
""" | ||
Generate a python class object given a class string | ||
|
||
Parameters | ||
---------- | ||
typename | ||
props | ||
description | ||
namespace | ||
|
||
Returns | ||
------- | ||
|
||
""" | ||
string = generate_class_string_r(typename, props, description, namespace) | ||
scope = {'Component': Component, '_explicitize_args': _explicitize_args} | ||
# pylint: disable=exec-used | ||
exec(string, scope) | ||
result = scope[typename] | ||
return result | ||
|
||
def required_props(props): | ||
""" | ||
|
@@ -507,6 +672,49 @@ def create_docstring(component_name, props, events, description): | |
for p, prop in list(filter_props(props).items())), | ||
events=', '.join(events)) | ||
|
||
def create_docstring_r(component_name, props, events, description): | ||
""" | ||
Create the Dash component docstring | ||
|
||
Parameters | ||
---------- | ||
component_name: str | ||
Component name | ||
props: dict | ||
Dictionary with {propName: propMetadata} structure | ||
events: list | ||
List of Dash events | ||
description: str | ||
Component description | ||
|
||
Returns | ||
------- | ||
str | ||
Dash component docstring | ||
""" | ||
# Ensure props are ordered with children first | ||
props = reorder_props(props=props) | ||
|
||
desctext = '' | ||
desctext += "\n".join( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No need to append, just do |
||
[('#\' @param {:s} {:s}'.format(p, props[p]['description'].replace('\n', ' '))) | ||
for p in props.keys() | ||
if not p.endswith("-*") and | ||
p not in keyword.kwlist and | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ |
||
p not in ['setProps']] | ||
) | ||
|
||
return('''#' {name} component | ||
#' @description See <https://developer.mozilla.org/en-US/docs/Web/HTML/Element/{name}> | ||
#' @export | ||
#' @param ... The children of this component and/or 'wildcards' of the form: `data-*` or `aria-*` | ||
{desctext} | ||
''' | ||
).format( | ||
name=component_name, | ||
description=description, | ||
desctext=desctext, | ||
events=', '.join(events)) | ||
|
||
def parse_events(props): | ||
""" | ||
|
@@ -550,6 +758,25 @@ def parse_wildcards(props): | |
list_of_valid_wildcard_attr_prefixes.append(wildcard_attr[:-1]) | ||
return list_of_valid_wildcard_attr_prefixes | ||
|
||
def parse_wildcards_r(props): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🐫 I do not think this function is necessary, it is the same as |
||
""" | ||
Pull out the wildcard attributes from the Component props | ||
|
||
Parameters | ||
---------- | ||
props: dict | ||
Dictionary with {propName: propMetadata} structure | ||
|
||
Returns | ||
------- | ||
list | ||
List of Dash valid wildcard prefixes | ||
""" | ||
list_of_valid_wildcard_attr_prefixes = [] | ||
for wildcard_attr in ["data-*", "aria-*"]: | ||
if wildcard_attr in props.keys(): | ||
list_of_valid_wildcard_attr_prefixes.append(wildcard_attr[:-1]) | ||
return list_of_valid_wildcard_attr_prefixes | ||
|
||
def reorder_props(props): | ||
""" | ||
|
@@ -837,3 +1064,8 @@ def js_to_py_type(type_object, is_flow_type=False, indent_num=0): | |
# All other types | ||
return js_to_py_types[js_type_name]() | ||
return '' | ||
|
||
def make_package_name(namestring): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Can it be |
||
# first, *rest = namestring.split('_') Python 3 | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ |
||
first, rest = namestring.split('_')[0], namestring.split('_')[1:] | ||
return first + ''.join(word.capitalize() for word in rest) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,12 @@ | ||
import collections | ||
import json | ||
import os | ||
#from dash.development.base_component import generate_class | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ |
||
from .base_component import generate_class | ||
from .base_component import generate_class_file | ||
|
||
from .base_component import generate_class_r | ||
from .base_component import generate_class_file_r | ||
#from dash.development.base_component import generate_class_file | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ⚡ |
||
|
||
def _get_metadata(metadata_path): | ||
# Start processing | ||
|
@@ -94,8 +97,15 @@ def generate_classes(namespace, metadata_path='lib/metadata.json'): | |
componentData['description'], | ||
namespace | ||
) | ||
generate_class_file_r( | ||
name, | ||
componentData['props'], | ||
componentData['description'], | ||
namespace | ||
) | ||
|
||
# Add an import statement for this component | ||
# RK: need to add import statements in R namespace file also | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is that still todo ? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I've added |
||
with open(imports_path, 'a') as f: | ||
f.write('from .{0:s} import {0:s}\n'.format(name)) | ||
|
||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can update this docstring to reflect that it generate R components.
docstring
,keywords arguments
,repr
are python things that may not relate to R.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, this makes sense. When I first started editing, I think I was trying too hard to maintain parity between the existing code and the R-related bits I introduced. I'll revise so that my additions reflect R conventions when appropriate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
P.S. Thanks for having a look!