Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Refactor Dash.jl to support same parameters as Dash for Python & R #12
Refactor Dash.jl to support same parameters as Dash for Python & R #12
Changes from all commits
a8530d0
122c49f
7acf2b4
7d3b6ea
c3bdc1a
f4cceac
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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.
@waralex I imagine this function updates the
layout
withinapp
to include the tree of components that are passed? If that's the case, doeslayout
support passing a function that returns a component?i.e. in both Python and R, I believe, the
layout
can be passed a tree of components or a single function. for example:or
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.
@rpkyle function is not supported yet, so you can add it to issues list so I don't forget it later
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.
@waralex Added in #15.
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.
@waralex I'm not sure why we changed the default for
url_base_pathname
from"/"
toNone
, but I'll investigate. The Python docstring still reflects"/"
, I suspect that this was never updated, so what you have here (nothing
) is likely correct.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.
@rpkyle See the comment to the previous issue
Also
This is why I don't like to write expanded docstrings in the initial stages of development. Because it need to be completely rewritten before release, or it will accumulate similar inconsistencies. With frequent changes to an api that has not yet stabilized it is very difficult to track that there are no inconsistencies in the code and documentation
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.
@waralex For these, specifying
nothing
as the default seems right, and then internally we want to make sure to resolve in the following orderDASH_APP_NAME
environment variable exists, use `//' if present, otherwise:DASH_URL_BASE_PATHNAME
environment variable exists, use if present, otherwise:url_base_pathname
, which defaults to"/"
when undefinedThere 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.
@rpkyle I removed the environment variables mention from docstring for a reason. Working with environment variables is one of the components of a future PR.
url_base_pathname
not defaults to "/" - this is a documentation error in python.routes_pathname_prefix
andrequests_pathname_prefix
defaults to "/" whenurl_base_pathname
is None, but if you set url_base_pathname to default "/" andrequests_pathname_prefix
to some value then you will get the error "You suppliedurl_base_pathname
andrequests_pathname_prefix
. This is ambiguous ..."