-
Notifications
You must be signed in to change notification settings - Fork 24
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
NYC_buildings: Modernize notebook #386
base: main
Are you sure you want to change the base?
Conversation
Your changes were successfully integrated in the dev site, make sure to review |
17121aa
to
9b6c32c
Compare
In this PR, pinning |
Can you please add more details on this? |
Ok it looks like a packaging issue. I don't understand why |
@Azaya89 can you maybe try to pin again |
This is not able to work because |
I would like to see it failing on the CI to see if it reports the same error that you get. |
nyc_buildings/anaconda-project.yml
Outdated
@@ -15,20 +15,26 @@ user_fields: [examples_config] | |||
|
|||
channels: | |||
- defaults |
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.
@Azaya89 ah I just noticed something. It's good practice not to mix the defaults
channel with conda-forge
. So when we use conda-forge
we should replace defaults
with nodefaults
, to avoid the defaults
channel to be added by default 🙃 Can you try that on your machine and re-lock?
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.
OK, but there was no failure in the CI here. This is diabolical!
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.
Done!
Your changes were successfully integrated in the dev site, make sure to review |
1 similar comment
Your changes were successfully integrated in the dev site, make sure to review |
Your changes were successfully integrated in the dev site, make sure to review |
fe3b8fb
to
b2030b9
Compare
Your changes were successfully integrated in the dev site, make sure to review |
@maximlt I think this PR is ready for another review with the following notes:
|
Ok thanks for the report. Depending on the performance issues, it might be that we end up not updating the code in this example. |
:( |
Isaiah reports that it takes about 30 seconds to run a cell with the full visualization with geopandas and that now (reverting back to spatialpandas on his local machine) it's taking even longer. |
b2030b9
to
3983383
Compare
This resolves all the previous issues about performance and reading of the |
This is a WIP PR to help with debugging the issues I'm having with this notebook. I also uploaded the new
.parq
file together with this PR since I don't have access to uploading it on AWS.Modernizing an example checklist
Preliminary checks
Change ‘anaconda-project.yml’ to use the latest workable version of packages
hvplot<0.9
tohvplot
,panel>=0.12,<1.0
topanel>=0.12
) of all other dependencies. Removing the upper pins of dependencies could necessitate code revisions in the notebooks to address any errors encountered in the updated environment. Should complexities or extensive time requirements arise, document issues for team discussion on whether to re-pin specific packages or explore other solutions.hvplot
tohvplot>=0.9.2
,hvplot>=0.8
tohvplot>=0.9.2
). Usually, the new/updated lower pin of a dependency will be the version resolved afteranaconda prepare
has been run. Execute!conda list
in a notebook, oranaconda run conda list
in the terminal, to display the version of each dependency installed in the environment. Adjusting the lower pin helps ensure that the locks produced for each platform (linux-64, win-64, osx-64, osx-arm64) rely on the tested dependencies and not on some older versions.Plot API updates (discussed on a per-example basis)
datashade
withrasterize
(read this page). Essentially,rasterize
allows Bokeh to handle the colormapping instead of Datashader.Interactivity API updates (discussed on a per-example basis)
pn.interact
usage.param.watch()
usage. This is pretty low-level and verbose approach and should not be used in Examples unless required, or an Example is specifically trying to demo its usage in an advanced workflow.pn.bind()
. Read this page for explanation.view()
method and call it directly, update the class by inheriting frompn.viewable.Viewer
and replaceview()
by__panel__()
. Here is an example.Panel App updates (discussed on a per-example basis)
pn.Column
, or more complicated to incorporate widgets, etc. Make the final app.servable()
.command: dashboard
declaration in theanaconda-project.yml
file), try adding it.template = pn.template.BootstrampTemplate
, but if building up an app across multiple cells, it is probably cleaner to declare the template at the top withpn.extension(template='bootstrap')
. See how to guide on setting a template.General code quality updates
warnings.simplefilter(‘ignore’)
somewhere at the start of the notebook, remove this line. Try to update the code to remove the warnings, if any. If updating the code to remove the warnings is taking significant amount of time and effort, bring it up for discussion and we may decide to disable warnings again.Text content
Visual appearance - Example
Visual appearance - Gallery
Ml Annotators
toML Annotators
), if not, add/update theexamples_config.title
field inanaconda-project.yml
description
field inanaconda-project.yml
Workflow (after you have made the changes above)
doit validate:<projectname>
doit test:<projectname>
doit doc_one –name <projectname>
. It’s better if the project notebook(s) is saved with its outputs (but be sure to clear outputs before committing to the examples repo!) when building the docs. Then open this file in your browser./builtdocs/index.html
and check how the site looks.