-
-
Notifications
You must be signed in to change notification settings - Fork 33
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
Dash R 0.9.1 release #272
Dash R 0.9.1 release #272
Conversation
Relocate metadata fixup logic from config.yml to R script
* Testing initial implementation * More testing * Callback Context Updates * Updating callback context logic * Fixing callback returns * Adding callback args conditional * Cleanup and additional changes to callback value conditionals * Comment cleanup * Added PMC callback validation, removed unnecessary code * Update R/dependencies.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update R/dependencies.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update R/dependencies.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update R/dependencies.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Added build to gitignore * Updated dependencies.R * Update boilerplate docs and add wildcard symbols * Drying up validation code and applying symbol logic * Update test to use symbols * Cleaned up code and added allsmaller test example * Cleaning up redundant code * Update FUNDING.yml * Updated callback_args logic and example * Adding basic unittests, updated validation * Fixed response for MATCH callbacks * Added integration test and updated examples for docs * Added additional integration test * Formatting and cleanup * update docs * Update to-do app * Add comments to examples * Change empy vector to character type. Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update boilerplate text. Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update tests/integration/callbacks/test_pattern_matching.py Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update tests/integration/callbacks/test_pattern_matching.py Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update tests/integration/callbacks/test_pattern_matching.py Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update tests/integration/callbacks/test_pattern_matching.py Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update tests/integration/callbacks/test_pattern_matching.py Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update tests/testthat/test-wildcards.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Update wildcards_test.R Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> * Removed triple colon syntax * Use seq_along and remove unnecessary unittest * Update CHANGELOG.md * Update CHANGELOG.md * Add support for arbitrary and sorted keys * Whitespace deleted * Added integration tests * Fixing test output * Fixing flakiness * Update test_pattern_matching.py * Update test_pattern_matching.py * Updating boilerplate text and test with generalized keys * Minor test fixes Co-authored-by: Ryan Patrick Kyle <rpkyle@users.noreply.github.com> Co-authored-by: Nicolas Kruchten <nicolas@plot.ly> Co-authored-by: rpkyle <ryan@plotly.com>
* Fixing NULL error with glue interpolation * Update utils.R * Update utils.R
* bump dash-renderer to v1.8.2 * Update CHANGELOG.md * add note about update to dash-renderer * Fixing flaky test
Co-authored-by: HammadTheOne <30986043+HammadTheOne@users.noreply.github.com>
Co-authored-by: HammadTheOne <30986043+HammadTheOne@users.noreply.github.com>
Co-authored-by: HammadTheOne <30986043+HammadTheOne@users.noreply.github.com>
Co-authored-by: HammadTheOne <30986043+HammadTheOne@users.noreply.github.com>
Co-authored-by: HammadTheOne <30986043+HammadTheOne@users.noreply.github.com>
Co-authored-by: HammadTheOne <30986043+HammadTheOne@users.noreply.github.com>
* Update setCallbackContext * Adding graphs test * Slight fix * bump version and update CHANGELOG * Less flaky test Co-authored-by: rpkyle <ryan@plotly.com>
Let me know when the package is ready for re-testing |
DESCRIPTION
Outdated
@@ -25,7 +25,7 @@ Imports: | |||
rlang, | |||
utils | |||
Suggests: | |||
testthat, | |||
testthat (>= 3.1.0), |
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.
I didn't even know there was a 3.1.0! Looks like it only got released a couple days ago. I think 3.0.0 would be just fine too
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.
testthat (>= 3.1.0), | |
testthat (>= 3.0.0), |
I see that all |
Concatenated components into package R scripts in cb93cdb. Note, this change requires the merge of plotly/dash#1798 to make this a streamlined process, and also address the
Updated in aff9b99.
Removed in 0ad3b24.
I think the most appropriate repo for this issue would be in
Removed in 68427b3.
Fixed with 68427b3. |
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.
Suggestions/comments below. There are also still a few dash:::
- in the test-components.R test file
DESCRIPTION
Outdated
@@ -25,7 +25,7 @@ Imports: | |||
rlang, | |||
utils | |||
Suggests: | |||
testthat, | |||
testthat (>= 3.1.0), |
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.
testthat (>= 3.1.0), | |
testthat (>= 3.0.0), |
R/utils.R
Outdated
if (!is.list(output[[1]])){output <- list(output)} | ||
invalid_outputs <- vapply(output, function(x) { | ||
!any(c('output', 'state') %in% attr(x, "class")) | ||
}, FUN.VALUE=logical(1)) | ||
|
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.
Are we sure this is the correct behaviour? I think outputs must only be of class "output", not "state"
R/utils.R
Outdated
# Verify that output contains no elements that are not either members of 'output' or 'state' classes | ||
if (any(invalid_outputs)) { | ||
stop(sprintf("Callback outputs must be outputs or states. Please verify formatting of callback outputs."), call. = FALSE) |
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.
If my above comment is correct, then this needs to update too
Thanks for the sharp eye and feedback @daattali! I think I've removed all the references to the internal dash namespace now, and I've updated the broken tests (which were setting a named list for the output instead of calling the output function itself). At Jack's suggestion, I'm also going to be adding the Dash Bootstrap Components library into the Dash R namespace. I'll do that in a separate PR, we're already pushing the scope of this one a little far. |
Awesome! Do the CI checks run the unit tests, and that's why they failed previously? It looks like there's a build step in CircleCI , I assume that part of that includes running the tests, but I'm not able to see it since I don't have access to that account. I think this PR is ready except for the breaking change of the order of parameters to dcc components changed. I think we should wait until we decide what to do about that, whether we'll fix it directly at the R level or higher up the chain |
* Adding dbc to dashR namespace * updated gitignore * Adding dbc docs and updating gulpfile * Updating test with dbc * Moved misc tests and added dbc snapshot * Fixing test * fixing id * Fixed export and test
@daattali I've reverted the prop order to the original unsorted order for the time being. We can talk about that more in detail next week, but if everything else looks good I think this should be ready to merge at this point. |
I'll test it again soon will let you know today |
This PR merges in the changes from
dev
tomaster
and updates the package, including the DESCRIPTION and CHANGELOG, in preparation for the Dash R 1.0 release to Github and CRAN.@daattali