Skip to content

improved .plot methods #925

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

Merged
merged 2 commits into from
Aug 6, 2021
Merged

Conversation

gdementen
Copy link
Contributor

I know the tests will fail because of the bogus Pandas doctest for plot.line but I would
like your advice on how to solve this. Do you know of a way in pytest to ignore one particular method doctests?

@gdementen gdementen requested a review from alixdamman March 2, 2021 16:34
@alixdamman
Copy link
Collaborator

Do you know of a way in pytest to ignore one particular method doctests?

Yes, this must be done in the setup.cfg file. You already have an example for the Array.astype method in this file:

--deselect larray/core/array.py::larray.core.array.Array.astype

Copy link
Collaborator

@alixdamman alixdamman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Few questions and suggestions.
The main suggestion would be to update the setup.cfg file to avoid to add # doctest : +SKIP everywhere in the Array.plot() examples.

@alixdamman
Copy link
Collaborator

LGTM by the way.

@gdementen gdementen added this to the 0.33 milestone Jul 30, 2021
@gdementen gdementen force-pushed the subplots=axisref branch 2 times, most recently from c507f74 to d5894aa Compare August 3, 2021 16:07
@gdementen gdementen changed the title implemented .plot(subplots=axis_ref) improved .plot methods Aug 3, 2021
@gdementen gdementen requested a review from alixdamman August 3, 2021 16:10
@gdementen
Copy link
Contributor Author

gdementen commented Aug 3, 2021

@alixdamman: I thought it would take me 2-3 hours to finish this, so that it can be included in the next release of larray and I can work on LIAM2 without interruption but it took me 5 days 😭. If I had known, I wouldn't have done it now but... so is life 🤷‍♂️.

@gdementen
Copy link
Contributor Author

@alixdamman: the review I am asking now is not about the code, but I'd be grateful if you could play around with it, to see if there are more bugs I didn't find. You could also check the release notes to see if it is understandable...

@gdementen
Copy link
Contributor Author

PS: sorry for the rebase-force-push but the code changed so much since the first version it did not make sense to make commits "from there"

@alixdamman
Copy link
Collaborator

If I may, the improvement of the plot function comes from a specific demand or your wish to encourage our users to stop using Excel for everything ? (there is no wrong answer)

@gdementen
Copy link
Contributor Author

There wasn't any specific demand (hence the remark that I wouldn't have done it now had I known it would take me that much time). FWIW, the subplots part is something I always found frustrating, and the rest are things I found while developing the subplots part. But this might help a bit with a vague demand I regularly receive of "better plotting tools". I never investiguated what each of these demands really mean. I just know bm wants to easily do more kinds of visualisations so that wouldn't qualify, but I think other members of the IO team also use misc Python visualization to explore their data, so this PR will probably help streamline that kind of workflow.

@gdementen
Copy link
Contributor Author

Also, a few improvements in there could be considered bug fixes as many plot submethods were not really usable as is and needed very convoluted ways to set them up including transpose, combine axes and explicit for loops to get something decent, so that's no wonder nobody used them 😄.

* support for subplots=axes, x=axes, y=axes and by=axes in plot functions
* support for labels (instead of axes) in x and y for line plot and scatter
* support legend kwargs
* better title when subplots is True
* default subplots layout to last axis horizontal
* pie legend defaults to False (it does not bring any more information than there already is)
@gdementen gdementen merged commit 93d3629 into larray-project:master Aug 6, 2021
@gdementen gdementen deleted the subplots=axisref branch August 6, 2021 09:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants