WIP Initial version of updated code for R transpiler #449

rpkyle · 2018-11-05T15:11:41Z

No description provided.

T4rk1n

Look good so far, couple things to remove as I don't think they translate from python to R.

Also, I'd like those methods (and the python generation too) to be in a separate file as it should be the file for the python component class and associated methods. But that can be in another PR.

T4rk1n · 2018-11-07T22:45:22Z

dash/development/base_component.py

+def generate_class_string_r(typename, props, description, namespace):
+    """
+    Dynamically generate class strings to have nicely formatted docstrings,
+    keyword arguments, and repr


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.

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.

P.S. Thanks for having a look!

T4rk1n · 2018-11-07T22:45:49Z

dash/development/base_component.py

+    # 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.


Remove those comments, they contains todo's that are specifics to python classes.

Will do. I'll also finish commenting my additions where appropriate.

T4rk1n · 2018-11-07T22:46:20Z

dash/development/base_component.py

+        component_name=typename,
+        props=filtered_props,
+        events=parse_events(props),
+        description=description).replace('\r\n', '\n')


I added that .replace('\r\n', '\n') for generating the docstring on windows because git doesn't auto replace them when they are in docstring. Not sure this problem is the same with R.

Actually, it is a consideration in R as well, though I think I should remove the \n and replace with a single space, since R's help viewer does its own line wrapping without adding linefeeds.

T4rk1n · 2018-11-07T22:46:30Z

dash/development/base_component.py

+        events=parse_events(props),
+        description=description).replace('\r\n', '\n')
+
+    # pylint: disable=unused-variable


T4rk1n · 2018-11-07T22:47:13Z

dash/development/base_component.py

+          '{:s}=NULL'.format(p))
+         for p in prop_keys
+         if not p.endswith("-*") and
+         p not in keyword.kwlist and


⚡ This keyword list filter is for python keywords, won't be the same in R.

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?

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?

If the function argument matches a reserved word (which is not first escaped with backticks), the R parser will throw an error:

Error: unexpected 'if' in "myfun <- function(if"

The help page (also produced when you type ?reserved) is here:

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

You can include those keywords in https://github.com/plotly/dash/blob/master/dash/development/_all_keywords.py as a new variable r_keywords and use it in this PR.

Great, will do.

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.

OK, I think I've made the minimal sufficient set of changes required to base_component.py necessary to load dashR with the current version of dash-html-components and dash-core-components. Mind having another look at the changes?

@rmarren1
@T4rk1n

T4rk1n · 2018-11-07T22:49:51Z

dash/development/base_component.py

@@ -445,6 +589,27 @@ def generate_class(typename, props, description, namespace):
    result = scope[typename]
    return result

+def generate_class_r(typename, props, description, namespace):


⚡ the generate_class for python is for dynamically loaded components, we don't do that in R.

T4rk1n · 2018-11-07T22:50:20Z

dash/development/base_component.py

+    props = reorder_props(props=props)
+
+    desctext = ''
+    desctext += "\n".join(


No need to append, just do desctext = "\n".join(

T4rk1n · 2018-11-07T22:50:31Z

dash/development/component_loader.py

@@ -1,9 +1,12 @@
 import collections
 import json
 import os
+#from dash.development.base_component import generate_class


T4rk1n · 2018-11-07T22:50:33Z

dash/development/component_loader.py

-
+from .base_component import generate_class_r
+from .base_component import generate_class_file_r
+#from dash.development.base_component import generate_class_file


T4rk1n · 2018-11-07T22:50:38Z

dash/development/component_loader.py


        # Add an import statement for this component
+        # RK: need to add import statements in R namespace file also


Is that still todo ?

I've added export() statements to the NAMESPACE file for the R package, don't think we'll need to provide an import statement. Will remove the comment.

rmarren1 · 2018-11-08T13:16:41Z

dash/development/base_component.py

+        [('#\' @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


rmarren1 · 2018-11-08T13:20:30Z

dash/development/base_component.py

@@ -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):


🐫 I do not think this function is necessary, it is the same as parse_wildcards currently. We can keep this as one function used by the R and Python generation until we have a reason to re-write it.

rmarren1 · 2018-11-08T13:24:19Z

dash/development/base_component.py

@@ -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):


Can it be make_package_name_r to be more explicit?

rmarren1 · 2018-11-08T13:27:08Z

dash/development/base_component.py

@@ -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):
+   # first, *rest = namestring.split('_') Python 3


rmarren1 · 2018-11-09T03:41:57Z

dash/development/_all_keywords.py

+# >>> import keyword
+# >>> keyword.kwlist
+
+kwlist = set([


We should change this to python_keywords then to be consistent. I think we should do it here, it's only 4 lines and I don't think it needs its own PR.

rmarren1 · 2018-11-09T03:42:55Z

dash/development/base_component.py

         p not in ['dashEvents', 'fireEvent', 'setProps']] + ['**kwargs']
    )

    required_args = required_props(props)
-    return c.format(**locals())
+    return c.format(typename=typename,


rpkyle · 2018-12-06T22:51:23Z

Closing this PR in favour of #483.

Ryan Patrick Kyle added 2 commits November 5, 2018 10:10

Initial version of updated code for R transpiler

e39bcf1

Fixed creation of R functions for components

23bb710

rpkyle changed the title ~~Initial version of updated code for R transpiler~~ WIP Initial version of updated code for R transpiler Nov 5, 2018

Fixed creation of R functions for components, triple quote issue

9ce9509

nicolaskruchten mentioned this pull request Nov 7, 2018

Combine extract-meta and component generation in a cli. #451

Merged

T4rk1n reviewed Nov 7, 2018

View reviewed changes

rmarren1 reviewed Nov 8, 2018

View reviewed changes

Edits related to PR comments

ed5d5db

rmarren1 reviewed Nov 9, 2018

View reviewed changes

Ryan Patrick Kyle and others added 17 commits November 9, 2018 09:44

Edited _all_keywords.py so R/Py kwlists similarly named.

3df1cd6

Updated base_component.py kwlist >> python_keywords

451283f

Attempting to resolve pylint warnings

5998545

Resolve pylint warning by removing unused generate_class_r code

6606943

Updated base_component.py

4575b9f

Updated component_loader.py

9bb7603

Merge branch 'master' into R

f1e47f5

Updated documentation for generate_classes_r

f4cd844

Updated base_component.py to support .Rd without roxygen2

aae11d6

Updated component_loader.py to import generate_help_file_r

62bf001

Updated component_loader.py and base_component.py

89b7b85

Modified base_component.py to address propNames resolution problem

664e516

Updated keyword list to include R reserved words

3bf21b3

Changes to support transpiling of JSON>>R within Python

84a6745

Changes to support transpiling of JSON>>R within Python

2a4ac5a

Restore export_string generation

3341d8e

Updates to component loader/base component

0d7943d

Ryan Patrick Kyle added 6 commits November 27, 2018 11:45

Merged RPK branch

e182736

Updates to generate_js_metadata_r

13d0d71

Removed external_url parsing

8a43d98

Clarified comments in code

8d2d538

Added statements to copy JS deps into inst/lib

008598f

Updated base_component.py

918a073

rpkyle closed this Dec 6, 2018

rpkyle mentioned this pull request Dec 10, 2018

[WIP] Revised version of R 'transpiler' with support for generator methods #483

Merged

rpkyle deleted the R branch February 5, 2019 21:41

AnnMarieW pushed a commit to AnnMarieW/dash that referenced this pull request Jan 6, 2022

Update webpack dependencies (plotly#449)

7e25f50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP Initial version of updated code for R transpiler #449

WIP Initial version of updated code for R transpiler #449

rpkyle commented Nov 5, 2018

T4rk1n left a comment

T4rk1n Nov 7, 2018

rpkyle Nov 8, 2018

rpkyle Nov 8, 2018

T4rk1n Nov 7, 2018

rpkyle Nov 8, 2018

T4rk1n Nov 7, 2018

rpkyle Nov 8, 2018

T4rk1n Nov 7, 2018

T4rk1n Nov 7, 2018

rmarren1 Nov 8, 2018

rpkyle Nov 8, 2018 •

edited

Loading

T4rk1n Nov 8, 2018

rpkyle Nov 8, 2018

rpkyle Nov 27, 2018

rpkyle Nov 28, 2018 •

edited

Loading

T4rk1n Nov 7, 2018

T4rk1n Nov 7, 2018

T4rk1n Nov 7, 2018

T4rk1n Nov 7, 2018

T4rk1n Nov 7, 2018

rpkyle Nov 8, 2018

rmarren1 Nov 8, 2018

rmarren1 Nov 8, 2018

rmarren1 Nov 8, 2018

rmarren1 Nov 8, 2018

rmarren1 Nov 9, 2018

rmarren1 Nov 9, 2018

rpkyle commented Dec 6, 2018


		# Add an import statement for this component
		# RK: need to add import statements in R namespace file also

WIP Initial version of updated code for R transpiler #449

WIP Initial version of updated code for R transpiler #449

Conversation

rpkyle commented Nov 5, 2018

T4rk1n left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpkyle Nov 8, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpkyle Nov 28, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rpkyle commented Dec 6, 2018

rpkyle Nov 8, 2018 •

edited

Loading

rpkyle Nov 28, 2018 •

edited

Loading