Skip to content

TST: Add test to verify DataFrame's constructor doesn't convert Nones to strings on string columns #40912

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

Closed

Conversation

cgarciae
Copy link

whatsnew

Adds a very simple test to verify that Nones are not casted to strings as "None" inside the DataFrame's constructor for string columns.

Questions:

  • Should the test be more general?
  • What is the proper location for the test?

@cgarciae cgarciae changed the title TST: Add test to verify DataFrame's constructor doesn't convert Nones to strings on string/object columns TST: Add test to verify DataFrame's constructor doesn't convert Nones to strings on string columns Apr 13, 2021
def test_consistency_of_string_columns_with_none(self):
# df created from list, None is casted to str
df = DataFrame(["1", "2", None], columns=["a"], dtype="str")
type(df.loc[2].values[0])
Copy link
Contributor

Choose a reason for hiding this comment

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

so this is not actually testing anything, instead you need to actually construct the expected frame and use assert_frame_equal. more to the point, I am pretty sure we have tests very similar to this. happy to take if we don't, but pls have a look (would take this if its slightly different than what we have, so pls give a look). we have a ton of tests so have to look around a bit.

Copy link
Author

@cgarciae cgarciae Apr 13, 2021

Choose a reason for hiding this comment

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

Hey @jreback, by accident I didn't push the next commit with some actual asserts (see update).
However, if there is already a similar test that resolves #32218 we can reference it and close this PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah if you'd have a look around. I recall a similar issue to this but nothing referenced, pls have a look see at the existing tests (I know there are a lot)

Copy link
Member

Choose a reason for hiding this comment

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

It was fixed in 1.2.0 (but broken in 1.1.5), but I don't directly find anything in the 1.2.0 notes about this (and also didn't directly find an issue about it). So I think it is fine to add the test to be safe.

@jreback jreback added Constructors Series/DataFrame/Index/pd.array Constructors Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate labels Apr 13, 2021
@github-actions
Copy link
Contributor

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label May 23, 2021
@simonjayhawkins simonjayhawkins added Needs Review Testing pandas testing functions or related to the test suite and removed Stale labels May 25, 2021
@simonjayhawkins
Copy link
Member

@cgarciae can you merge master to resolve conflicts and can take a look

@jreback
Copy link
Contributor

jreback commented Sep 1, 2021

closing as stale

@jreback jreback closed this Sep 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Constructors Series/DataFrame/Index/pd.array Constructors Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent behavior when DataFrame with strings and None is created from lists or dictionary.
4 participants