Refactor to plotting API following pandas/matplotlib implementation #473
Conversation
There was a problem hiding this comment.
Overall, very excellent PR, @danielhuppmann ! I really like the implementation (I assume similar/inspired by pandas?). I will take a look through the website update as well, but otherwise, this is g2g from my side!
| @@ -116,6 +116,11 @@ def read_pandas(path, default_sheet='data', *args, **kwargs): | |||
| if len(xl.sheet_names) > 1 and 'sheet_name' not in kwargs: | |||
| kwargs['sheet_name'] = default_sheet | |||
| df = pd.read_excel(path, *args, **kwargs) | |||
|
|
|||
There was a problem hiding this comment.
Is this change related to the PR? Could not figure out why.
There was a problem hiding this comment.
Yes, this is not related to the PR per se, but fixes a pandas v1.2 issue on the fly.
I started working on a nightly test suite so that we don't catch these incompatibilities while working on new features, but can resolve them in mini-PRs when they arise...
| pd.testing.assert_frame_equal(df.timeseries().reset_index(), | ||
| tdf, check_like=True) | ||
|
|
||
| # check that timeseries data is as expected |
There was a problem hiding this comment.
Is this change related to the PR? Could not figure out why.
There was a problem hiding this comment.
As with the other comment, this resolves a pandas v1.2 issue on the fly.
|
I went through the docs, only thing I could find as an issue was a broken link to the plotting gallery (the last link in the picture points to here: https://pyam-iamc.readthedocs.io/en/plotting-plot-accessor/examples/index.html) |
Thanks for catching this, fixed there and also in the readme and tutorial notebooks... (shouldn't have entered that rabbit hole in the first place). |
|
Cool! I think option 3 will be very useful. Will try to test this soon (am on phone). One thing that i think would be helpful would be to make the docstring available when accessing .plot() etc instead of referring to pyam.plotting... I guess that is the correct place for it and you don't want to have to copy/paste text leading to inconsistencies, but it does slow down the coding |
|
re @byersiiasa
Fully agree that this would be the right way for - but given that this also wasn't done properly in the previous implementation, I'll make an issue and not tackle it in this PR. |
Please confirm that this PR has done the following:
Tests AddedName of contributors Added to AUTHORS.rstDescription of PR
The current plotting API (e.g.,
df.bar_plot()) follows neither the pandas/matplotlib convention (df.plot.bar()) nor the seaborn package (sns.barplot(df)). This PR implements aPlotAccessorclass following the pandas package.This allows to access plots via three options:
IamDataFrame.plot.<kind>(**kwargs)IamDataFrame.plot(kind='<kind>', **kwargs)pyam.plotting.<kind>(df, **kwargs)The third option will take not just an IamDataFrame but also suitable pandas.DataFrame instances. This would make it easier for related packages (open-scm, silicone) to use the pyam-plotting library without casting to an IamDataFrame.
The PR also cleans up the documentation pages and updates the examples & tutorials.
See the rendered docs pages of this PR here.
closes #474