Skip to content

Conversation

@rpkyle
Copy link
Contributor

@rpkyle rpkyle commented Jul 26, 2019

This PR proposes to add functionality for storing R examples for help files in the dash-info.yaml file, as outlined in #831.

Closes #831.

@rpkyle rpkyle added this to the Dash v1.1.0 milestone Jul 26, 2019
@rpkyle rpkyle requested a review from byronz July 26, 2019 19:13
@rpkyle rpkyle requested a review from alexcjohnson as a code owner July 26, 2019 19:13
@rpkyle rpkyle self-assigned this Jul 26, 2019
@rpkyle
Copy link
Contributor Author

rpkyle commented Jul 26, 2019

@byronz Not sure why it passed tests locally but failed on CircleCI, but made a minor change and it looks 👌 now.

description=description.replace('\n', ' ')
))
if rpkg_data is not None:
if rpkg_data is not None and rpkg_data.get('r_examples') is not None:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🐄 minor: 'r_examples' in rpkg_data

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That does look cleaner! Will edit and test; thanks for the feedback.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in a7883da

result = ""
if the_ex and "code" in the_ex.keys():
result += wrap("examples",
wrap("dontrun" if the_ex["dontrun"] else "",
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if the_ex.get("dontrun")
When would a user want or not want dontrun?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a slight delay when executing inline examples within R packages; CRAN requires that each example run and exit without errors in a second or two. Dash apps aren't typically terminated until the user interrupts them; this is problematic for CRAN (and devtools) checks.

In addition, CRAN checks will fail if a dependency does not exist in the repository when examples are executed. Setting \dontrun to TRUE allows the package author to include examples but specify that they not be executed during package tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My apologies for not catching the initial comment previously, change has been made!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in cf0d476

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no worries, the way I wrote it did look like I was quoting you rather than suggesting a change.

@alexcjohnson
Copy link
Collaborator

@rpkyle this looks good - just two small changes as requested by me (before my question about dontrun was a suggested syntax change) and by byron.

@rpkyle rpkyle requested review from alexcjohnson and byronz August 1, 2019 18:26
os.makedirs('R')
if os.path.isfile("dash-info.yaml"):
with open("dash-info.yaml") as yamldata:
rpkg_data = yaml.safe_load(yamldata)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need a fallback declaration of rpkg_data if dash-info.yaml doesn't exist - like we had previously:

else:
    rpkg_data = None

(alternatively, initialize it to None up above the if os.path.isfile("dash-info.yaml") - just need to ensure it always exists by the time we ask for it below)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch. Sometimes I wish there was a ⚾️ mitt emoji.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 139527d

@rpkyle rpkyle requested a review from alexcjohnson August 1, 2019 23:58
Copy link
Collaborator

@alexcjohnson alexcjohnson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💃

@rpkyle rpkyle merged commit 2bc4987 into master Aug 2, 2019
@rpkyle rpkyle deleted the issue831-inline-r-examples branch August 2, 2019 14:37
HammadTheOne pushed a commit to HammadTheOne/dash that referenced this pull request May 28, 2021
HammadTheOne pushed a commit that referenced this pull request Jul 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants