Skip to content

TST: add test for Index.where when needing to cast to object dtype #40888

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
wants to merge 3 commits into from

Conversation

cgarciae
Copy link

@cgarciae cgarciae commented Apr 11, 2021

Hey! This is my first PR with actual code: added a very basic test to validate #32413.

Questions:

  • What is the best place for this test? (currently put it in pandas/tests/indexes/test_indexing.py)
  • Should I try to create another test that uses the index input argument?

@@ -248,6 +249,20 @@ def test_putmask_with_wrong_mask(self, index):
index.putmask("foo", fill)


class TestCasting:
def test_maybe_cast_with_dtype(self):
Copy link
Member

Choose a reason for hiding this comment

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

This test is better suited in pandas/tests/indexes/base_class/test_where.py

Copy link
Author

Choose a reason for hiding this comment

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

The main issue was the casting so I was not sure, I could move it anyway.

Copy link
Member

Choose a reason for hiding this comment

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

Thanks, Generally we group tests by the method calls on the particular objects (Index.where)

@mroeschke mroeschke added the Testing pandas testing functions or related to the test suite label Apr 11, 2021
@simonjayhawkins simonjayhawkins added this to the 1.3 milestone Apr 12, 2021
@jreback jreback changed the title Adds a test for #32413 Object dtype gives ValueError in _maybe_cast_with_dtype Apr 12, 2021
@jorisvandenbossche jorisvandenbossche changed the title Object dtype gives ValueError in _maybe_cast_with_dtype TST: Index.where works when needing to cast to object dtype Apr 12, 2021
@jorisvandenbossche jorisvandenbossche changed the title TST: Index.where works when needing to cast to object dtype TST: add test for Index.where when needing to cast to object dtype Apr 12, 2021
Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

@cgarciae Thanks for the PR!

(side-note: for future reference, try to give your PRs a descriptive title; only reference the issue number is not that useful (since we don't know by heart what those numbers are about ;))

@cgarciae
Copy link
Author

Thanks @jorisvandenbossche and @mroeschke for the feedback! I'll clean the PR a bit :)

Copy link
Member

@jorisvandenbossche jorisvandenbossche left a comment

Choose a reason for hiding this comment

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

As a last comment, could you still move the test?
I think tests/indexes/numeric/test_indexing.py might be best suites, there is a TestWhere class

@cgarciae
Copy link
Author

@jorisvandenbossche moved the test there!

@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.

@@ -376,6 +376,18 @@ def test_where(self, klass, index):
result = index.where(klass(cond))
tm.assert_index_equal(result, expected)

def test_maybe_cast_with_dtype(self):
Copy link
Member

Choose a reason for hiding this comment

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

can you give this a name of the form test_where_[...] ideally more desc

@jbrockmendel
Copy link
Member

the underlying problem in #32413 was at a lower level IIRC. should we have a test on astype or a constructor?

@jreback jreback removed this from the 1.3 milestone May 26, 2021

fixed_index = index.where(cond, other)

tm.assert_index_equal(other, Index(["a0", "a1"]))
Copy link
Contributor

Choose a reason for hiding this comment

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

can you use

result =
expected =
tm.assert_index_equal(result, expected)


fixed_index = index.where(cond, other)

tm.assert_index_equal(other, Index(["a0", "a1"]))
Copy link
Contributor

Choose a reason for hiding this comment

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

pls parameterize if you have multiple cases

@simonjayhawkins
Copy link
Member

@cgarciae can you address @jreback comments.

@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
Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Object dtype gives ValueError in _maybe_cast_with_dtype
7 participants