-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Add dataset line plot #4820
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
Draft
Illviljan
wants to merge
29
commits into
pydata:main
Choose a base branch
from
Illviljan:Illviljan-dataset_line_plot
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Add dataset line plot #4820
Changes from all commits
Commits
Show all changes
29 commits
Select commit
Hold shift + click to select a range
3557771
Add dataset line plot
Illviljan fd822cd
Update dataset_plot.py
Illviljan 516c0d0
Update dataset_plot.py
Illviljan da30687
Handle when hue is not None
Illviljan 7127229
sort and add linewidth
Illviljan 133ccd5
variant with dataarray
Illviljan 295352a
Use the dataarray variant.
Illviljan 798d2b9
Merge branch 'master' into Illviljan-dataset_line_plot
Illviljan 4f1ddc2
copy doc from dataarray
Illviljan ed31db7
Update dataset_plot.py
Illviljan fb7e9db
Update dataset_plot.py
Illviljan e10d5ca
allow adding any number of extra coords
Illviljan 474928f
Explain how ds will becom darray
Illviljan 54bd639
Update dataset_plot.py
Illviljan f58c52a
Merge branch 'master' into Illviljan-dataset_line_plot
Illviljan a8edc08
Update dataset_plot.py
Illviljan 9c416f2
use coords for coords
Illviljan 3ade6c4
Merge branch 'master' into Illviljan-dataset_line_plot
Illviljan c14ae47
Merge branch 'master' into Illviljan-dataset_line_plot
Illviljan 93b50e2
Merge branch 'master' into Illviljan-dataset_line_plot
Illviljan 7110220
Explain goal of moving ds plots to da
Illviljan 76d42e5
Merge branch 'master' into Illviljan-dataset_line_plot
Illviljan b8c749d
Merge branch 'main' into Illviljan-dataset_line_plot
Illviljan 3e90c1f
Merge branch 'main' into Illviljan-dataset_line_plot
Illviljan bf8b0ea
handle non-existant coords
Illviljan 424a006
Update dataset_plot.py
Illviljan fe5bece
Look through the kwargs to find extra coords
Illviljan 5f57f5c
Merge branch 'main' into Illviljan-dataset_line_plot
Illviljan 1dfad9f
improve ds to da wrapper
Illviljan File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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.
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.
Adding an explanation here how the dataset becomes a dataarray should help making sense of the autogenerated docs I think.
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.
Does the
_dsplot
decorator not work for this case? It would help minimize the changes.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.
Now that the line plot comes from the dataarray side you don't have to do all the additional things
_dsplot
is doing. All that's really needed then is to attach to the plot class and then do this tweak to the docs so that they make more sense.My end goal here is to move all the dataset specific plots to the dataarray side and use this thin wrapper for dataset plotting. Back when I started this PR in january there was only
scatter
so it made more sense back then.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.
Thanks @Illviljan Can you add this to the docstring of
_attach_to_plot_class
Let's also see what @mathause thinks...