Docstring default values + minor corrections #782

shammamah-zz · 2019-06-19T21:42:05Z

Contributor Checklist

I have broken down my PR scope into the following TODO tasks
- Add default values to generated docstrings, if they are defined
- Ensure that component names that start with a vowel are preceded by "An" and not "A"
- Remove duplicate periods in top-level component descriptions, and add periods if they are missing
- Change pluralization for "list of" prop when it's a list of dict (as opposed to an "s" being appended after all of the keys are defined for a dict)
I have run the tests locally and they passed. (refer to testing section in contributing)
I have added tests, or extended existing tests, to cover any new features or bugs fixed in this PR

optionals

I have added entry in the CHANGELOG.md
If this PR needs a follow-up in dash docs, community thread, I have mentioned the relevant URLS as follow
- this github #PR number updates the dash docs
- here is the show and tell thread in plotly dash community

Remove string comparison to number.

Add 'dict' to all props that are objects in metadata_test.

…fied. Replace 'optional' with default values for props that have them specified in metadata_test.

…ngs. Fix pylint errors.

alexcjohnson · 2019-06-21T00:14:46Z

dash/development/_py_components_generation.py


 Keyword arguments:\n{args}"""
    ).format(
+        n='n' if component_name[0].lower() in ['a', 'e', 'i', 'o', 'u']
+        else '',


Classy touch 🥂

alexcjohnson · 2019-06-21T00:17:00Z

dash/development/_py_components_generation.py

@@ -249,6 +251,8 @@ def create_docstring(component_name, props, description):
                else prop['flowType'],
                required=prop['required'],
                description=prop['description'],
+                default=prop['defaultValue']['value']
+                if 'defaultValue' in prop.keys() else '',


🐄 'defaultValue' in prop is sufficient, .keys() is unneeded

this gets repeated a few times - 🌴 ?

I'm a bit conflicted about moving this out into a function or something. It would need to be at the topmost scope, and only gets used three times. Is there another way to do this without repeating myself?

(The .keys() have been removed in 9271f5b and f5bbab4 (I found another instance of it being used, so I went ahead and fixed that too).)

I tried doing this, just for brevity: bbccf47

I probably would have just made a top-level function but that's OK, we can leave it inline. I don't think bbccf47 is an improvement - shorter in one case, longer in the others, and less readable.

Though, the other thing you could do is move this logic into create_prop_docstring - ie pass the whole object in rather than the string value.

The prop objects wouldn't have been trivial to extract in situations like this:

dash/dash/development/_py_components_generation.py

Line 245 in cfa120e

args='\n'.join(

So I compromised with 20cf393!

dash/development/_py_components_generation.py

…r they contain them.

Only add 's' to end if referring to a non-dict type; otherwise, change 'dict' to 'dicts' and don't put an 's' on the end. Fix pylint error.

Remove all periods from the end of description, if there are any, and add in another one.

alexcjohnson

Nice job! 💃

* Changed window.location for absolute paths * Added integration tests * Flaky test run 3.7

Shammamah Hossain added 4 commits June 19, 2019 13:22

Change 'A' to 'An' for component names that begin with vowels.

7e96e57

Add support for adding default values to the end of the docstring.

cf13ccb

Handle periods for nested props, and clean up default value logic.

ec642f1

Put default values next to the type definition in docstring.

31fe48f

shammamah-zz requested review from alexcjohnson and byronz as code owners June 19, 2019 21:42

Shammamah Hossain added 2 commits June 20, 2019 14:47

Add prop type specification for nested props (PropTypes.shape).

31210a7

Merge branch 'master' into docstring-default-values

dd99c0d

shammamah-zz force-pushed the docstring-default-values branch from 3b6b683 to 584341c Compare June 20, 2019 20:02

Fix pylint errors.

6ae9004

Remove string comparison to number.

shammamah-zz force-pushed the docstring-default-values branch from 584341c to 6ae9004 Compare June 20, 2019 20:09

Shammamah Hossain added 2 commits June 20, 2019 17:46

Add 'dict' to all props that are objects.

cc2253a

Add 'dict' to all props that are objects in metadata_test.

Replace 'optional' with default values for props that have them speci…

60809ab

…fied. Replace 'optional' with default values for props that have them specified in metadata_test.

shammamah-zz force-pushed the docstring-default-values branch from 6783169 to 29266ce Compare June 20, 2019 21:50

Shammamah Hossain added 2 commits June 20, 2019 17:54

Add more logic to adding period to the end of descriptions in docstri…

77754e1

…ngs. Fix pylint errors.

Remove unnecessary comma.

db2230b

shammamah-zz force-pushed the docstring-default-values branch from 29266ce to db2230b Compare June 20, 2019 21:54

Merge branch 'master' into docstring-default-values

9fa5f62

alexcjohnson reviewed Jun 21, 2019

View reviewed changes

dash/development/_py_components_generation.py Outdated Show resolved Hide resolved

Shammamah Hossain added 8 commits June 21, 2019 10:03

Remove '.keys()' when searching prop dictionary.

9271f5b

Remove unnecessary '.keys()'.

f5bbab4

Add 'default' to docstring.

7c6d72b

Check whether default values equal null/empty types instead of whethe…

c70659e

…r they contain them.

Make default value logic shorter.

bbccf47

Add more logic for 'list of' types.

56d984d

Only add 's' to end if referring to a non-dict type; otherwise, change 'dict' to 'dicts' and don't put an 's' on the end. Fix pylint error.

Simplify logic for adding periods onto ends of descriptions.

37043d8

Remove all periods from the end of description, if there are any, and add in another one.

Move logic for determining default value into create_prop_docstring.

20cf393

shammamah-zz force-pushed the docstring-default-values branch from 3189ad8 to 20cf393 Compare June 21, 2019 19:33

Shammamah Hossain added 2 commits June 21, 2019 15:34

Merge branch 'master' into docstring-default-values

0e090f3

Remove periods from empty descriptions.

bf78b45

shammamah-zz requested a review from alexcjohnson June 21, 2019 20:02

alexcjohnson approved these changes Jun 22, 2019

View reviewed changes

shammamah-zz merged commit ba27feb into master Jun 22, 2019

shammamah-zz deleted the docstring-default-values branch June 22, 2019 01:29

This was referenced Jun 26, 2019

Dash Bio Python components require docstrings formatted like those from other libraries plotly/dash-bio#378

Closed

Update dashBio package for compatibility with dash 0.1.0 plotly/dash-bio#382

Merged

shammamah-zz mentioned this pull request Aug 2, 2019

0.1.4 release plotly/dash-bio#412

Closed

6 tasks

shammamah-zz mentioned this pull request Sep 17, 2019

Dashbio tests refactoring plotly/dash-bio#411

Merged

6 tasks

HammadTheOne added a commit to HammadTheOne/dash that referenced this pull request May 28, 2021

Change dcc.Link for absolute paths (plotly#782)

4adc07e

* Changed window.location for absolute paths * Added integration tests * Flaky test run 3.7

HammadTheOne added a commit that referenced this pull request Jul 23, 2021

Change dcc.Link for absolute paths (#782)

c39aba6

* Changed window.location for absolute paths * Added integration tests * Flaky test run 3.7

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Docstring default values + minor corrections #782

Docstring default values + minor corrections #782

shammamah-zz commented Jun 19, 2019 •

edited

Loading

alexcjohnson Jun 21, 2019

alexcjohnson Jun 21, 2019

alexcjohnson Jun 21, 2019

shammamah-zz Jun 21, 2019

shammamah-zz Jun 21, 2019 •

edited

Loading

shammamah-zz Jun 21, 2019

alexcjohnson Jun 21, 2019

shammamah-zz Jun 21, 2019

alexcjohnson left a comment

Docstring default values + minor corrections #782

Docstring default values + minor corrections #782

Conversation

shammamah-zz commented Jun 19, 2019 • edited Loading

Contributor Checklist

optionals

alexcjohnson Jun 21, 2019

Choose a reason for hiding this comment

alexcjohnson Jun 21, 2019

Choose a reason for hiding this comment

alexcjohnson Jun 21, 2019

Choose a reason for hiding this comment

shammamah-zz Jun 21, 2019

Choose a reason for hiding this comment

shammamah-zz Jun 21, 2019 • edited Loading

Choose a reason for hiding this comment

shammamah-zz Jun 21, 2019

Choose a reason for hiding this comment

alexcjohnson Jun 21, 2019

Choose a reason for hiding this comment

shammamah-zz Jun 21, 2019

Choose a reason for hiding this comment

alexcjohnson left a comment

Choose a reason for hiding this comment

shammamah-zz commented Jun 19, 2019 •

edited

Loading

shammamah-zz Jun 21, 2019 •

edited

Loading