Skip to content

CLN: de-duplicate numeric type check in _libs/testing #36625

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

Merged
merged 10 commits into from
Oct 6, 2020

Conversation

arw2019
Copy link
Member

@arw2019 arw2019 commented Sep 25, 2020

  • closes #xxxx
  • tests added / passed
  • passes black pandas
  • passes git diff upstream/master -u -- "*.py" | flake8 --diff
  • whatsnew entry

In testing.pyx we keep a list of numeric dtypes and use that for type checking. But we have functions that do this in tslibs/util.pxd so it's better to use those instead


cdef bint is_comparable_as_number(obj):
return isinstance(obj, NUMERIC_TYPES)
cdef bint is_comparable_as_real_number(obj):
Copy link
Contributor

Choose a reason for hiding this comment

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

is wouldn't even do this, just use the imported one directly.

Copy link
Member

Choose a reason for hiding this comment

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

not a huge deal, but unless we expect to use it elsewhere, id rather this new func live here rather than in util; otherwise it gets re-compiled into basically every cython module.

@jreback jreback added Clean Testing pandas testing functions or related to the test suite labels Sep 26, 2020
@jreback jreback added this to the 1.2 milestone Sep 26, 2020
@jreback
Copy link
Contributor

jreback commented Sep 26, 2020

looks good, still some lint issues, ping on green (small comment)

@arw2019
Copy link
Member Author

arw2019 commented Sep 26, 2020

Green

@jbrockmendel
Copy link
Member

@arw2019 can you merge master

Copy link
Contributor

@jreback jreback left a comment

Choose a reason for hiding this comment

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

can merge on green

@arw2019
Copy link
Member Author

arw2019 commented Oct 5, 2020

Sorry to have been quiet!

Merged again just now - all green

@jreback jreback merged commit f1adcd1 into pandas-dev:master Oct 6, 2020
@jreback
Copy link
Contributor

jreback commented Oct 6, 2020

thanks @arw2019

kesmit13 pushed a commit to kesmit13/pandas that referenced this pull request Nov 2, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Clean Testing pandas testing functions or related to the test suite
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants