Fixes to address unintended props reshuffling behaviour #545

rpkyle · 2019-01-18T23:11:05Z

Proposed changes introduced by this PR:

The ordering of props in Dash components is maintained by providing object_pairs_hook=OrderedDict to json.loads .
Handling of prop_keys exclusions has been refactored to account for removal of events, and to properly omit wildcards.
R components now require children to be passed as a list or function, as in Dash for Python, and ellipsis is used for wildcards only, as the final formal argument for component functions.
Component library dependencies are now copied to inst/deps rather than inst/lib to address an issue with install_github.
R documentation now has additional whitespace to improve readability.
R (overall) package help files are now available.
R component handling of defaultValue fields is now similar to Dash for Python, fixing issues with rendering of components due to passing incorrect defaults.
Removed dependency of JavaScript metadata generation for DashR on version.py.
Use modified version of Mark Amery's byteify function to avoid storing strings from JSON as Unicode in Python 2.x, so that lists of strings in components should be identical in appearance across versions.
A new function assert_valid_wildcards is called when wildcards are available for a given component, to validate any wildcard input passed in via ... in the corresponding function formals in R.

alexcjohnson · 2019-01-19T00:36:58Z

.gitignore

@@ -20,3 +20,4 @@ dist
 npm-debug*
 /.tox
 .idea
+package-lock.json


It shouldn't be necessary to ignore this, though there are occasional issues.

K, this has been 🔪 ... as has the package-lock.json.

T4rk1n

I don't understand the package-lock.json, there's no package.json in dash. 🔪

T4rk1n

Need to fix the error in generate_class_string

dash/development/_r_components_generation.py

T4rk1n · 2019-01-21T20:51:33Z

dash/development/_r_components_generation.py

@@ -163,7 +142,7 @@ def generate_class_string(name, props, project_shortname, prefix):
        '\'{}\''.format(p)
        for p in prop_keys
        if '*' not in p and
-        p not in ['setProps', 'dashEvents', 'fireEvent']
+        p != 'setProps' + ['**kwargs']


That doesn't work.

>>> 'setProps' + ['**kwargs'] Traceback (most recent call last): File "<input>", line 1, in <module> TypeError: must be str, not list

Suggested change

p != 'setProps' + ['**kwargs']

p not in ['setProps', '**kwargs']

Oh that's not quite it either... the p not in [...] or p != ... clause is a filter in an iterator - in the python generator code:

dash/dash/development/_py_components_generation.py

Lines 118 to 126 in e72f483

default_argtext += ", ".join(

[('{:s}=Component.REQUIRED'.format(p)

if props[p]['required'] else

'{:s}=Component.UNDEFINED'.format(p))

for p in prop_keys

if not p.endswith("-*") and

p not in python_keywords and

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

)

note the extra [...] from immediately after .join( to right before + ['**kwargs'] that turns this into a list that we can then append '**kwargs' to by adding a second list. I'm assuming in R **kwargs doesn't mean anything, but there's some other feature to play the role of allowing arbitrary extra named arguments?

I'm assuming in R **kwargs doesn't mean anything, but there's some other feature to play the role of allowing arbitrary extra named arguments?

@alexcjohnson Correct. For the wildcards, I've used ellipsis (aka \dots, ...) for this purpose. It seems to be working to pass in an arbitrary number (including zero) of wildcard arguments.

alexcjohnson · 2019-01-21T22:59:18Z

I don't understand the package-lock.json, there's no package.json in dash. 🔪

Hah too funny! Right, dash is the one repo that doesn't use npm at all. Oh well, sounds like @rpkyle sorted this out :)

rpkyle · 2019-01-22T16:18:54Z

I don't understand the package-lock.json, there's no package.json in dash. hocho

Hah too funny! Right, dash is the one repo that doesn't use npm at all. Oh well, sounds like @rpkyle sorted this out :)

I'm glad I've somehow fallen into the component management pipeline on the Python side as well, because for all the bumps and bruises, I'm certainly learning a few things.

🐛 modified prop_names address trailing whitespace 🔧 flake8 and pylint edits 🚿 pylint edits

🚿 pylint edit 🚿 flake8 edit 🚿 flake8 edit 🐛 fixed TypeError

rpkyle · 2019-01-22T21:27:46Z

Use modified version of Mark Amery's byteify function to avoid storing strings from JSON as Unicode in Python 2.x, so that lists of strings in components should be identical in appearance across versions.

If it's possible to have a look at my recent changes and let me know if they are acceptable, and what further refinements are needed, that'd be great.

N.B. I ran into an error while testing the R component generation in Python 3 I hadn't seen previously:

File "dash/dash/development/_r_components_generation.py", line 282, in write_help_file for item in prop_keys[:]: TypeError: 'odict_keys' object is not subscriptable

I attempted to address this by wrapping props.keys() with list;

dash/dash/development/_r_components_generation.py

Line 134 in 01b6d9d

prop_keys = list(props.keys())

dash/dash/development/_r_components_generation.py

Line 280 in 01b6d9d

prop_keys = list(props.keys())

Component generation appears to be successful for both languages, and using Python 2 and 3; will look a bit more tonight to confirm.

@alexcjohnson @T4rk1n @rmarren1

alexcjohnson · 2019-01-23T20:56:21Z

I attempted to address this by wrapping props.keys() with list;

Yeah, .keys() and various other things return iterators instead of lists in py3, that one continually trips me up / annoys me... you've got the correct solution.

T4rk1n · 2019-01-23T20:59:48Z

File "dash/dash/development/_r_components_generation.py", line 282, in write_help_file for item in prop_keys[:]: TypeError: 'odict_keys' object is not subscriptable

Yes, dict.keys() returns a list in python 2 and a generator in python 3 so you need to cast to list.

T4rk1n

💃

T4rk1n · 2019-01-25T23:34:58Z

dash/development/component_generator.py

@@ -82,13 +89,17 @@ def generate_components(components_source, project_shortname,
    )

    with open(os.path.join(project_shortname, 'metadata.json'), 'w') as f:
-        json.dump(metadata, f)
+        json.dump(metadata, f, sort_keys=True)


⏪ Sorting the key reorder the keys, the ordered dict will keep the right order alone.

rpkyle · 2019-01-29T10:36:39Z

@T4rk1n If PRs #439 and #91 look ready to go, mind quickly checking this one also? It should be somewhat straightforward.

set default type of Input components to 'text'

Ryan Patrick Kyle added 21 commits January 11, 2019 20:34

fixes to address default values, support for source maps

d208936

adjust R package dependencies

af56606

added file to .gitignore

96c69c1

corrected error in help

c8573ad

inserted whitespace as suggested by alexcjohnson

34837df

fixed deps naming issue for install_github

e53cca2

added import for _get_metadata

45a7dcd

restored _get_metadata call for JSON imports

9fe81f4

fixed .Rbuildignore strings

04d2b3f

fix for lone flake8 error

f54fee1

replaced second occurrence of json.loads() with _get_metadata

624c389

addressed flake8 errors

5498307

modifed metadata load path

aa8c6dc

replaced missing parenthesis

74a4d5b

flake8 passes

bcb3d48

attempting alternative approach for JSON>>OrderedDict

5a4c5fb

corrected inst/lib to inst/deps

3eca5be

removed import of _get_metadata

be0646b

pylint and flake8 passing

85ada0c

removed spurious ref to metadata

d5fdc19

refactored prop_keys exclusion

c90d0e5

rpkyle self-assigned this Jan 18, 2019

rpkyle requested review from Marc-Andre-Rivet, T4rk1n and rmarren1 January 18, 2019 23:11

pylint fix for indentation

4e4a57a

alexcjohnson reviewed Jan 19, 2019

View reviewed changes

T4rk1n reviewed Jan 21, 2019

View reviewed changes

Ryan Patrick Kyle added 2 commits January 21, 2019 14:53

edits to fully address 🔪 events

c20ced1

updated .gitignore

b9552ab

T4rk1n requested changes Jan 21, 2019

View reviewed changes

rpkyle force-pushed the RPK2 branch from 9e6f236 to 2dbfa43 Compare January 22, 2019 19:59

Ryan Patrick Kyle added 2 commits January 22, 2019 15:03

↔️ added byteify fn to prevent str >> unicode

8aa3f25

🐛 modified prop_names address trailing whitespace 🔧 flake8 and pylint edits 🚿 pylint edits

2️⃣3️⃣ attempt to preserve version compatibility

5e251f0

🚿 pylint edit 🚿 flake8 edit 🚿 flake8 edit 🐛 fixed TypeError

rpkyle force-pushed the RPK2 branch from 2dbfa43 to 5e251f0 Compare January 22, 2019 20:04

Ryan Patrick Kyle added 3 commits January 22, 2019 15:17

🐛 revised byteify to use items() vs iteritems in 3️⃣

1aeb507

🔨 rewrote code to only use byteify in 2️⃣

a334699

prop_keys as list to handle TypeError for odict_keys in 3️⃣

01b6d9d

T4rk1n approved these changes Jan 23, 2019

View reviewed changes

alexcjohnson mentioned this pull request Jan 24, 2019

No events plotly/dash-core-components#444

Merged

T4rk1n reviewed Jan 25, 2019

View reviewed changes

Ryan Patrick Kyle and others added 9 commits January 25, 2019 22:12

⏪ do not set sort_keys

2251b30

Merge branch 'master' into RPK2

4ee280b

🔨 wildcards only available when props contain -*

9108f1e

Merge branch 'RPK2' of github.com:plotly/dash into RPK2

a97d523

updated R component generation to properly provide package help

38d693e

👕 flake8 edits

3499cd8

🔨 version information now sourced consistently

a622ccb

👕 pylint ws removal

817bfb0

updated R package description to note two langs

c65084f

rpkyle merged commit ab4f708 into master Jan 29, 2019

alexcjohnson deleted the RPK2 branch January 31, 2019 23:29

alexcjohnson mentioned this pull request Feb 1, 2019

use OrderedDict in byteify #561

Merged

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

Merge pull request plotly#545 from plotly/input-dflt-type

94844b6

set default type of Input components to 'text'

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fixes to address unintended props reshuffling behaviour #545

Fixes to address unintended props reshuffling behaviour #545

rpkyle commented Jan 18, 2019 •

edited

Loading

alexcjohnson Jan 19, 2019

rpkyle Jan 21, 2019 •

edited

Loading

T4rk1n left a comment

T4rk1n left a comment

T4rk1n Jan 21, 2019

alexcjohnson Jan 21, 2019

rpkyle Jan 22, 2019

alexcjohnson commented Jan 21, 2019

rpkyle commented Jan 22, 2019

rpkyle commented Jan 22, 2019

alexcjohnson commented Jan 23, 2019

T4rk1n commented Jan 23, 2019

T4rk1n left a comment

T4rk1n Jan 25, 2019

rpkyle commented Jan 29, 2019 •

edited

Loading

	p != 'setProps' + ['**kwargs']
	p not in ['setProps', '**kwargs']

	default_argtext += ", ".join(
	[('{:s}=Component.REQUIRED'.format(p)
	if props[p]['required'] else
	'{:s}=Component.UNDEFINED'.format(p))
	for p in prop_keys
	if not p.endswith("-*") and
	p not in python_keywords and
	p not in ['dashEvents', 'fireEvent', 'setProps']] + ['**kwargs']
	)

Fixes to address unintended props reshuffling behaviour #545

Fixes to address unintended props reshuffling behaviour #545

Conversation

rpkyle commented Jan 18, 2019 • edited Loading

alexcjohnson Jan 19, 2019

Choose a reason for hiding this comment

rpkyle Jan 21, 2019 • edited Loading

Choose a reason for hiding this comment

T4rk1n left a comment

Choose a reason for hiding this comment

T4rk1n left a comment

Choose a reason for hiding this comment

T4rk1n Jan 21, 2019

Choose a reason for hiding this comment

alexcjohnson Jan 21, 2019

Choose a reason for hiding this comment

rpkyle Jan 22, 2019

Choose a reason for hiding this comment

alexcjohnson commented Jan 21, 2019

rpkyle commented Jan 22, 2019

rpkyle commented Jan 22, 2019

alexcjohnson commented Jan 23, 2019

T4rk1n commented Jan 23, 2019

T4rk1n left a comment

Choose a reason for hiding this comment

T4rk1n Jan 25, 2019

Choose a reason for hiding this comment

rpkyle commented Jan 29, 2019 • edited Loading

rpkyle commented Jan 18, 2019 •

edited

Loading

rpkyle Jan 21, 2019 •

edited

Loading

rpkyle commented Jan 29, 2019 •

edited

Loading