-
-
Notifications
You must be signed in to change notification settings - Fork 193
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
Circos: App Improvements #72
Conversation
Addresses: #64 |
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.
t would be helpful for reviewers if the name of a given input/state is coherent across the different callbacks it is used in, i.e. for example State("output-data-upload", "children") is named uploadData on line 548 and data on line 516.
It would also be helpful to add docstrings to the callback briefly describing what the callback is used for or what should happen in general in the callback (this is also useful to achieve documentation driven development ^^)
As these are mainly suggestions for improvements of the code layout, you can choose to implement them (or not ;)) in a subsequent PR to keep the PR short, so I approved it.
[Input("circos-tabs", "value")], | ||
[State("previous-tab", "children")] | ||
) | ||
def store_previous_tab(tabs, prev_tab): |
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.
you could use a dcc.Store component for that
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.
Tried this out. It seems to be super slow, for my application, so I'll just stick to the current method. Thanks for telling me about this neat feature though 👍
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.
In the custom tab, the label "Max" of the slider goes over the border, this is minor, maybe you should merge this PR first so it is ready for the demo :)
Dash Bio pull request
Name and description of your component
Main libraries used in component
Before asking for a review
PR merging checklist
Please make sure you have done these things before asking for approval to finally merge.
master
: checkout the master branch withgit checkout master
, pull from latest mastergit pull
, checkout to your branchgit checkout your-branch
, merge master into your branchgit merge master
. Resolve conflicts.package-lock.json
(runnpm install
)requirements.txt
is completenpm run build:all
ornpm run build:js
followed bynpm run build:py
python setup.py sdist
, then copydist/dash_bio-X.Y.Z.tar.gz
into the top-level directory)pip install dash-bio-X.Y.Z.tar.gz
)dash-bio
folder by the above command(s)DDS Deployment
developers
user credentials from a dash-bio team member.git pull
).git remote -v
, make sure you have the dash-bio one:https://dash-gallery.plotly.host/GIT/dash-bio
.git remote add dash-bio https://dash-gallery.plotly.host/GIT/dash-bio
.developers
user credentials, have them ready.git push dash-bio master
.DDS Deployment Debugging
git push dash-bio master
you may encounter the following issue:This may be because the
git
credentials are saved on theclient-side
. In order to solve this, do the following:Windows
OS X
resolve: #59