Skip to content

Plotting Int64 columns with nulled integers (NAType) fails #32073 #32387

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 28 commits into from
Closed

Plotting Int64 columns with nulled integers (NAType) fails #32073 #32387

wants to merge 28 commits into from

Conversation

jeandersonbc
Copy link
Contributor

@MarcoGorelli
Copy link
Member

Thanks @jeandersonbc

However, before this PR can be accepted, you need to write tests / make sure you don't break existing ones - see contributing to the code base:

pandas is serious about testing and strongly encourages contributors to embrace test-driven development (TDD). This development process “relies on the repetition of a very short development cycle: first the developer writes an (initially failing) automated test case that defines a desired improvement or new function, then produces the minimum amount of code to pass that test.” So, before actually writing any code, you should write your tests. Often the test can be taken from the original GitHub issue. However, it is always worth considering additional use cases and writing corresponding tests.

Adding tests is one of the most common requests after code is pushed to pandas. Therefore, it is worth getting in the habit of writing tests ahead of time so this is never an issue.

@jeandersonbc
Copy link
Contributor Author

Thanks for the feedback @MarcoGorelli! I'm wondering whether the example provided in the related issue (first comment in #32073) would be sufficient to be added as a test in the TestDataFramePlots (test_frame.py) class. I started the discussion on tests on the issue yesterday (although here might be more appropriated).

TomAugspurger
TomAugspurger reviewed Mar 2, 2020
View reviewed changes
Copy link
Contributor

@TomAugspurger TomAugspurger left a comment

Choose a reason for hiding this comment

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

Thanks for working on this. We'll need a test and a release note in 1.1.0.rst.

pandas/plotting/_matplotlib/core.py
Comment on lines +419 to +421
if values.isna().any().all():
values = values.astype(float)

Copy link
Contributor

Choose a reason for hiding this comment

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

It seems like date-like data can be included here, and we don't want to convert those to floats.

I think this should be restricted to

if is_integer_dtype(values.dtype):
    values = values.to_numpy(dtype="float", na_value=np.nan)

jbrockmendel and others added 24 commits March 3, 2020 11:44
@jbrockmendel @jeandersonbc
REF: collect+parametrize reorder_levels tests (#32373)
26d3297
@rushabh-v @jeandersonbc
TST: Allow definition of pd.CategoricalDtype with a specific `categ…
0ff6b96 
…ories.dtype` (#32115)
@jbrockmendel @jeandersonbc
TYP: annotations for internals, set_axis (#32376)
64f76e9
@jbrockmendel @jeandersonbc
misplaced DataFrame.join test (#32375)
70b840b
@Iqrar99 @jeandersonbc
DOC: Fixed ES01, PR07, SA04 error in pandas.core.groupby.DataFrameGro…
8262397 
…upBy.shift (#32356)
@TomAugspurger @jeandersonbc
BUG: Pickle NA objects (#32104)
d2d14b7 
According to
https://docs.python.org/3/library/pickle.html#object.__reduce__,

> If a string is returned, the string should be interpreted as the name
> of a global variable. It should be the object’s local name relative to
> its module; the pickle module searches the module namespace to determine
> the object’s module. This behaviour is typically useful for singletons.

Closes #31847
@AdrianMastronardi @jeandersonbc
DOC: Fix SA04 errors in docstrings #28792 (#32182)
ba7895e
@jbrockmendel @jeandersonbc
CLN: remove _igetitem_cache (#32319)
f2f8605
@jbrockmendel @jeandersonbc
Avoid unnecessary values_from_object (#32398)
6a48be6
@jbrockmendel @jeandersonbc
ENH: infer freq in timedelta_range (#32377)
bc4c189
@jbrockmendel @jeandersonbc
BUG: 2D DTA/TDA arithmetic with object-dtype (#32185)
6023860
@jbrockmendel @simonjayhawkins @jeandersonbc
TST: broken off from #32187 (#32258)
66422c4 
Co-authored-by: Simon Hawkins <[email protected]>
@jbrockmendel @jeandersonbc
REF: simplify PeriodIndex._shallow_copy (#32280)
7210810
@jbrockmendel @jeandersonbc
CLN: setitem_with_indexer cleanups (#32341)
e262a71
@jbrockmendel @jeandersonbc
BUG: None / Timedelta incorrectly returning NaT (#32340)
143faa0
@SaturnFromTitan @jeandersonbc
TST: Using more fixtures in of tests/base/test_ops.py (#32313)
d2413f9
@jbrockmendel @jeandersonbc
CLN: remove unused values from interpolate call (#32400)
0a7ebfd
@ShaharNaveh @jeandersonbc
CLN: some code cleanups to pandas/_libs/missing.pyx (#32367)
3f9b4e8
@s-scherrer @jeandersonbc
BUG: fixes bug when using sep=None and comment keyword for read_csv (#…
0d41a23 
…31667)
@dsaxton @jeandersonbc
Don't create _join_functions (#32336)
a57de43
@a-y-khan @jeandersonbc
API: replace() should raise an exception if invalid argument is given (…
5759ad9 
…#31946)
@dsaxton @jeandersonbc
BUG: Fix __ne__ comparison for Categorical (#32304)
1c5b03f
@simonjayhawkins @jeandersonbc
CLN: clean-up show_versions and consistently use null for json output (…
b03a910 
…#32042)
@jeandersonbc
Add missing newline (#32404)
1973ddb
@jeandersonbc
Copy link
Contributor Author

well, I'm going to close this pull request and work in a single branch as this was not the first time that I messed up after updating my branch

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@TomAugspurger TomAugspurger TomAugspurger left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Plotting Int64 columns with nulled integers (NAType) fails