-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Pre-release packaging improvements for Dash for R #783
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
Changes from 14 commits
708e82d
4e71941
63e8132
363c95e
2cb4739
9ba7c6a
3111a09
8e29ca1
320e52e
02afe2a
9cee1d3
0f4ed2d
6eb6f04
b104e7a
e28724f
fcd82a5
1578e9a
c6a013a
8f50b8a
628e00d
4e2b859
393ab8b
5c1a104
7a8da3b
2d31fdc
857bb91
9b812ce
f0bfd07
862fe70
7ae5710
e35f040
15cbea8
4bea826
23d8b79
1c56d9a
e029904
77d6580
d53959f
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 |
|---|---|---|
|
|
@@ -18,16 +18,18 @@ | |
| # code below | ||
| r_component_string = """{funcname} <- function({default_argtext}{wildcards}) {{ | ||
| {wildcard_declaration} | ||
| props <- list({default_paramtext}{wildcards}) | ||
| if (length(props) > 0) {{ | ||
| props <- props[!vapply(props, is.null, logical(1))] | ||
| }} | ||
| component <- list( | ||
| props = list({default_paramtext}{wildcards}), | ||
| props = props, | ||
| type = '{name}', | ||
| namespace = '{project_shortname}', | ||
| propNames = c({prop_names}{wildcard_names}), | ||
| package = '{package_name}' | ||
| ) | ||
|
|
||
| component$props <- filter_null(component$props) | ||
|
|
||
| structure(component, class = c('dash_component', 'list')) | ||
| }} | ||
| """ # noqa:E501 | ||
|
|
@@ -42,7 +44,7 @@ | |
| version = "{project_ver}", src = list(href = NULL, | ||
| file = "deps"), meta = NULL, | ||
| script = {script_name}, | ||
| stylesheet = {css_name}, head = NULL, attachment = NULL, package = "{rpkgname}", | ||
| stylesheet = "{css_name}", head = NULL, attachment = NULL, package = "{rpkgname}", | ||
| all_files = FALSE), class = "html_dependency")""" # noqa:E501 | ||
|
|
||
| frame_body_template = """`{project_shortname}` = structure(list(name = "{project_shortname}", | ||
|
|
@@ -83,15 +85,15 @@ | |
| Authors @R: as.person(c({package_author})) | ||
| Description: {package_description} | ||
| Depends: R (>= 3.0.2){package_depends} | ||
| Imports: dash{package_imports} | ||
| Imports: {package_imports} | ||
| Suggests: {package_suggests} | ||
| License: {package_license} | ||
| URL: {package_url} | ||
| BugReports: {package_issues} | ||
| Encoding: UTF-8 | ||
| LazyData: true | ||
| Author: {package_author_no_email} | ||
| Maintainer: {package_author} | ||
| Maintainer: {maintainer} | ||
| """ | ||
|
|
||
| rbuild_ignore_string = r"""# ignore JS config files/folders | ||
|
|
@@ -103,6 +105,11 @@ | |
| .builderrc | ||
| .eslintrc | ||
| .npmignore | ||
| .editorconfig | ||
| .eslintignore | ||
| .prettierrc | ||
| .circleci | ||
| .github | ||
|
|
||
| # demo folder has special meaning in R | ||
| # this should hopefully make it still | ||
|
|
@@ -159,35 +166,25 @@ def generate_class_string(name, props, project_shortname, prefix): | |
| wildcards = "" | ||
| wildcard_declaration = "" | ||
| wildcard_names = "" | ||
| default_paramtext = "" | ||
| default_argtext = "" | ||
| accepted_wildcards = "" | ||
|
|
||
| if any("-*" in key for key in prop_keys): | ||
| accepted_wildcards = get_wildcards_r(prop_keys) | ||
| wildcards = ", ..." | ||
| wildcard_declaration = ( | ||
| "\n wildcard_names = names(assert_valid_wildcards(...))\n" | ||
| ) | ||
| wildcard_declaration = """ | ||
| wildcard_names = names(dash_assert_valid_wildcards(attrib = list({}), ...)) | ||
| """.format(accepted_wildcards.replace("-*", "")) # noqa:E501 | ||
| wildcard_names = ", wildcard_names" | ||
|
|
||
| default_paramtext = "" | ||
| default_argtext = "" | ||
| default_wildcards = "" | ||
|
|
||
| # Produce a string with all property names other than WCs | ||
| prop_names = ", ".join( | ||
| "'{}'".format(p) | ||
| for p in prop_keys | ||
| if "*" not in p and p not in ["setProps"] | ||
| ) | ||
|
|
||
| # in R, we set parameters with no defaults to NULL | ||
| # Here we'll do that if no default value exists | ||
| default_wildcards += ", ".join("'{}'".format(p) | ||
| for p in prop_keys if "*" in p) | ||
|
|
||
| if default_wildcards == "": | ||
| default_wildcards = "NULL" | ||
| else: | ||
| default_wildcards = "c({})".format(default_wildcards) | ||
|
|
||
| # 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": | ||
|
|
@@ -278,10 +275,10 @@ def generate_js_metadata(pkg_data, project_shortname): | |
| elif len(alldist) == 1: | ||
| rpp = alldist[0]["relative_package_path"] | ||
| if "css" in rpp: | ||
| css_name = rpp | ||
| css_name = "'{}'".format(rpp) | ||
| script_name = "NULL" | ||
| else: | ||
| script_name = rpp | ||
| script_name = "'{}'".format(rpp) | ||
| css_name = "NULL" | ||
| function_frame_body = frame_body_template.format( | ||
| project_shortname=project_shortname, | ||
|
|
@@ -323,6 +320,9 @@ def write_help_file(name, props, description, prefix): | |
|
|
||
| has_wildcards = any("-*" in key for key in prop_keys) | ||
|
alexcjohnson marked this conversation as resolved.
Outdated
|
||
|
|
||
| if has_wildcards: | ||
| wildcards = get_wildcards_r(prop_keys) | ||
|
|
||
| # 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": | ||
|
|
@@ -340,20 +340,21 @@ def write_help_file(name, props, description, prefix): | |
| ) | ||
|
|
||
| if has_wildcards: | ||
| item_text += '\n\n\\item{...}{wildcards: `data-*` or `aria-*`}' | ||
| default_argtext += ', ...' | ||
| item_text += """ | ||
| \n\n\\item{{...}}{{wildcards allowed have the form: `{}`}} | ||
| """.format(wildcards) | ||
|
|
||
| # in R, the online help viewer does not properly wrap lines for | ||
| # the usage string -- we will hard wrap at 80 characters using | ||
| # textwrap.fill, starting from the beginning of the usage string | ||
| argtext = prefix + name + "({})".format(default_argtext) | ||
|
|
||
| file_path = os.path.join('man', file_name) | ||
| with open(file_path, 'w') as f: | ||
| f.write(help_string.format( | ||
| funcname=format_fn_name(prefix, name), | ||
| name=name, | ||
| default_argtext=textwrap.fill(argtext, | ||
| default_argtext=textwrap.fill(default_argtext, | ||
| width=80, | ||
| break_long_words=False), | ||
| item_text=item_text, | ||
|
|
@@ -417,6 +418,26 @@ def write_js_metadata(pkg_data, project_shortname): | |
| ) | ||
| file_name = "internal.R" | ||
|
|
||
| # import dash_filter_null and dash_assert_valid_wildcards | ||
| component_helpers = """ | ||
| dash_assert_valid_wildcards <- function (attrib = list("data", "aria"), ...) | ||
| { | ||
| args <- list(...) | ||
| validation_results <- lapply(names(args), function(x) { | ||
| grepl(paste0("^", attrib, "-[a-zA-Z0-9]{1,}$", collapse = "|"), | ||
| x) | ||
| }) | ||
| if (FALSE %in% validation_results) { | ||
| stop(sprintf("The following wildcards are not currently valid in Dash: '%s'", | ||
| paste(names(args)[grepl(FALSE, unlist(validation_results))], | ||
| collapse = ", ")), call. = FALSE) | ||
| } | ||
| else { | ||
| return(args) | ||
| } | ||
| } | ||
| """ # noqa:E501 | ||
|
|
||
| # the R source directory for the package won't exist on first call | ||
| # create the R directory if it is missing | ||
| if not os.path.exists("R"): | ||
|
|
@@ -425,6 +446,7 @@ def write_js_metadata(pkg_data, project_shortname): | |
| file_path = os.path.join("R", file_name) | ||
| with open(file_path, "w") as f: | ||
| f.write(function_string) | ||
| f.write(component_helpers) | ||
|
Collaborator
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.
Contributor
Author
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. Good catch. Will fix.
Contributor
Author
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. fixed in c6a013a
Collaborator
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. Somehow this change (only adding
Contributor
Author
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. Yes. I am uncertain about how to check for wildcards without passing props to methods that ordinarily would not require them; would making
Collaborator
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 just looped through the metadata for wildcards again -> 15cbea8 |
||
|
|
||
| # now copy over all JS dependencies from the (Python) components dir | ||
| # the inst/lib directory for the package won't exist on first call | ||
|
|
@@ -478,7 +500,7 @@ def generate_rpkg( | |
| package_depends = ", " + package_depends.strip(",").lstrip() | ||
|
|
||
| if package_imports: | ||
| package_imports = ", " + package_imports.strip(",").lstrip() | ||
| package_imports = package_imports.strip(",").lstrip() | ||
|
|
||
| if package_suggests: | ||
| package_suggests = package_suggests.strip(",").lstrip() | ||
|
|
@@ -506,6 +528,8 @@ def generate_rpkg( | |
|
|
||
| package_author_no_email = package_author.split(" <")[0] + " [aut]" | ||
|
|
||
| maintainer = pkg_data.get("maintainer", pkg_data.get("author")) | ||
|
|
||
| if not (os.path.isfile("LICENSE") or os.path.isfile("LICENSE.txt")): | ||
| package_license = pkg_data.get("license", "") | ||
| else: | ||
|
|
@@ -540,7 +564,7 @@ def generate_rpkg( | |
| f2.write(rbuild_ignore_string) | ||
|
|
||
| # Write package stub files for R online help, generate if | ||
| # dashHtmlComponents or dashCoreComponents; makes it easy | ||
| # dashHtmlComponents, dashTable, or dashCoreComponents; makes it easy | ||
| # for R users to bring up main package help page | ||
| pkg_help_header = "" | ||
|
|
||
|
|
@@ -560,6 +584,15 @@ def generate_rpkg( | |
| written and maintained by the Dash team, is available in\n\ | ||
| the dashCoreComponents package. The source for this package\n\ | ||
| is on GitHub: plotly/dash-core-components." | ||
| if package_name in ["dashTable"]: | ||
| pkg_help_header = "Core Interactive Table Component for Dash" | ||
| pkg_help_desc = "Dash DataTable is an interactive table component\n\ | ||
| designed for viewing, editing, and exploring large datasets. DataTable is\n\ | ||
| rendered with standard, semantic HTML <table/> markup, which makes it\n\ | ||
| accessible, responsive, and easy to style. This component was written\n\ | ||
| from scratch in React.js specifically for the Dash community. Its API\n\ | ||
| was designed to be ergonomic and its behavior is completely customizable\n\ | ||
| through its properties." | ||
|
alexcjohnson marked this conversation as resolved.
Outdated
|
||
|
|
||
| description_string = description_template.format( | ||
| package_name=package_name, | ||
|
|
@@ -573,6 +606,7 @@ def generate_rpkg( | |
| package_url=package_url, | ||
| package_issues=package_issues, | ||
| package_author_no_email=package_author_no_email, | ||
| maintainer=maintainer, | ||
| ) | ||
|
|
||
| with open("DESCRIPTION", "w") as f3: | ||
|
|
@@ -821,3 +855,13 @@ def create_prop_docstring_r(prop_name, type_object, required, description, | |
| ': {}'.format(description) if description != '' else '' | ||
| ), | ||
| is_required='required' if required else 'optional') | ||
|
|
||
|
|
||
| def get_wildcards_r(prop_keys): | ||
| wildcards = "" | ||
| wildcards += ", ".join("'{}'".format(p) | ||
| for p in prop_keys if "*" in p) | ||
|
alexcjohnson marked this conversation as resolved.
Outdated
|
||
|
|
||
| if wildcards == "": | ||
| wildcards = "NULL" | ||
| return wildcards | ||
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.
also not new in this PR, but
item in r_keywordsworries 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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added a warning if we drop something from
r_keywords, in e35f040We 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.