Pre-release packaging improvements for Dash for R#783
Conversation
rpkyle
changed the title
|
|
||
| # Filter props to remove those we don't want to expose | ||
| for item in prop_keys[:]: | ||
| if item.endswith("-*") or item in r_keywords or item == "setProps": |
There was a problem hiding this comment.
also not new in this PR, but item in r_keywords worries me to just be silently dropped - if nothing else it seems like this should log a warning. Do any of our own components use R keywords?
There was a problem hiding this comment.
Added a warning if we drop something from r_keywords, in e35f040
We should keep an eye out for this when rebuilding all the components - if we never see this warning, perhaps we can lock the R keywords out by turning the warning into an error.
| file_path = os.path.join("R", file_name) | ||
| with open(file_path, "w") as f: | ||
| f.write(function_string) | ||
| f.write(component_helpers) |
There was a problem hiding this comment.
Good catch. Will fix.
There was a problem hiding this comment.
Somehow this change (only adding dash_assert_valid_wildcards to internal.R if needed by a component) was reverted - was that intended?
There was a problem hiding this comment.
Yes. I am uncertain about how to check for wildcards without passing props to methods that ordinarily would not require them; would making has_wildcards a global variable be appropriate in this case? I would like to avoid refactoring too much this week, I still have a bit to do outside of Python code maintenance.
There was a problem hiding this comment.
I just looped through the metadata for wildcards again -> 15cbea8
rpkyle
changed the title
|
I can't figure out what's going on with the python 3 tests here - some sort of error deep in Flask... what changed? I notice BTW that our requirements call for FWIW I see these same errors running the tests locally, yet in the same environment I can run a normal dash app just fine. @byronz can you take a look? |
| import functools | ||
|
|
||
| import pkg_resources | ||
| import yaml |
There was a problem hiding this comment.
@rpkyle I think the performance import pattern in official yaml doc is still valid.
try:
from yaml import CLoader as Loader, CDumper as Dumper
except ImportError:
from yaml import Loader, Dumper
requires-install.txt
Outdated
|
|
||
| # dash.testing | ||
| pytest | ||
| pytest<5.0.0 |
There was a problem hiding this comment.
temporary ⬇️ pytest version until the fix in flask is released pallets/flask#3278
@rpkyle the unit tests all passed in circleci now
| item_text += '\n\n\\item{...}{wildcards: `data-*` or `aria-*`}' | ||
| if any(key.endswith("-*") for key in prop_keys): | ||
| default_argtext += ', ...' | ||
| item_text += wildcard_help_template.format(get_wildcards_r(prop_keys)) |
There was a problem hiding this comment.
🐄 minor: always try to use list for str operation += might be expensive if it's in a big loop
| write_js_metadata(pkg_data, project_shortname, has_wildcards) | ||
|
|
||
| with open("NAMESPACE", "w") as f: | ||
| with open("NAMESPACE", "w+") as f: |
There was a problem hiding this comment.
🐮 minor it's personal preference, but I would avoid f with numbers, maybe for example fp_ns, fp_desr
3 participants
This PR proposes several modifications to the R package generator invoked by
dash-generate-components:assert_valid_wildcards(usenames(args)instead ofargs)data-*andaria-*assert_valid_wildcardswithdash_assert_valid_wildcardsin generated componentsfilter_nullin generated components, inline the logic provided by this functionImports:field, as only theDepends:field has a hardcoded entry (for the R binary itself)Maintainer:fielddash-info.yamldashpackage (CLI arguments todash-generate-componentsmay be used instead)Closes #777, related to #788.