-
Notifications
You must be signed in to change notification settings - Fork 118
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
Update expected figures to use matplotlib 3.5 #590
Update expected figures to use matplotlib 3.5 #590
Conversation
Codecov Report
@@ Coverage Diff @@
## main #590 +/- ##
=====================================
Coverage 94.2% 94.2%
=====================================
Files 50 50
Lines 5331 5331
=====================================
Hits 5022 5022
Misses 309 309
Continue to review full report at Codecov.
|
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 for fixing the nightlies! Small question in review.
@@ -437,7 +437,7 @@ def stack( | |||
|
|||
def as_series(index, name): | |||
_idx = [i[0] for i in index] | |||
return pd.Series([0] * len(index), index=_idx, name=name) | |||
return pd.Series([0] * len(index), index=_idx, name=name, dtype="int") |
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.
Not clear why this is needed, can you let me know why?
Can this be sent through calling code? e.g., either with
- move this
dtype
up as a kwarg or - add generic
**kwargs
which then passes through to this series call
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.
This is a function only used within this function (see just the following line) to select the rows where a timeseries crosses zero. Adding the dtype explicitly is done just to silence a warning in the log.
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.
Gotcha - won't hold up. But I assume this will not work for non-integer times?
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.
facepalm reread, and realize no issue here. sorry!
Please confirm that this PR has done the following:
Documentation AddedName of contributors Added to AUTHORS.rstDescription of PR
Alerted by the nightly tests... It seems that matplotlib v3.5 changed the order of the legends in stackplot-figures with totals. This PR simply updates the test-figures to reflect the new behavior (alternative approach: change the way that the "total" is added to the legend).
In addition, the PR silences a warning in the plotting module and changes the readme and install-bat-file from
py.test
topytest
(because this seems to be the more common usage).