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
Add bokeh plots and deprecate contributions_pie #715
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesYou may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation |
@dbrakenhoff shall I extend the plotly notebook example and add bokeh there as well, and rename to interactive_plots.ipynb? |
@dbrakenhoff this PR is ready for Review. But perhaps you can have a quick look at the Plotly plots in the notebook, somehow they don't show up on my system? Cheers, |
@raoulcollenteur, both are working for me in jupyter lab. Which versions are you on? For me. the following worked:
FYI, there seems to be some funkiness going on in the bokeh plots: And a note for anyone trying to view the bokeh plots in a VS Code notebook environment, the interactive figures don't show up. |
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.
See earlier comment about the funkiness in Bokeh plot. Otherwise looking good!
Setting up somewhat complicated shared axes seems a lot easier in bokeh than in plotly (not having to dive down into the low-level API). And I'm a fan of the range slider at the bottom :). Maybe we can just leave the background of that plot white, so the selected range (also gray) is clearer.
Hmm, interesting... Was this in VS code or JupyterLab? Do you have bokeh-jupyter-bokeh extension installed in JupyterLab? Perhaps that is necessary? |
This was in JupyterLab. Both without and with the bokeh-jupyter-bokeh jupyterlab extension installed. I think the data isn't sorted properly when I run |
Hmm, so the problem is not in bokeh plots. If I run the code above my data is well ordered. Python version: 3.11.3 Is this a new Pandas problem?! |
That could well be it, pandas 2.2 here. |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferencesYou may notice some variations in coverage metrics with the latest Coverage engine update. For more details, visit the documentation |
I think it is related to this issue: pandas-dev/pandas#57006 Probably good to fix in ml.get_out_series anyway.. Could you check if adding sort=True in pd.concat fixes the problem? |
Yep, that fixes it! Committed the change. Is it still working on your end? |
Yes all good here! I'll go through the changes in 2.2 again to see if more might be up. Should we merge this PR now? |
I updated the docs (https://pastas--715.org.readthedocs.build/en/715/api/generated/generated/generated/pastas.plotting.bokeh.Bokeh.results.html#pastas.plotting.bokeh.Bokeh.results) which I think looks better now. Somehow GH requires you need to re-review the PR @dbrakenhoff. |
Short Description
This PR adds basic plotting using Bokeh to Pastas models. To use this, bokeh plots needs to be registered as an extension, as implemented by @dbrakenhoff for plotly. I started with two basic plots, but this can grow over time.
I also added the deprecation for contributions_pie.
Checklist before PR can be merged: