-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
issue#27482 - added a check for if obj is instance of type in _isna-new #27664
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
issue#27482 - added a check for if obj is instance of type in _isna-new #27664
Conversation
Can someone help me; when I run |
Needs a test, don’t worry too much about the git thing |
could you point me to the file where the test would go for this? |
I had a quick look, would it be okay to go in this function: https://github.com/pandas-dev/pandas/blob/master/pandas/tests/dtypes/test_missing.py#L81 |
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 logic needs to be replicated in _isnew_old as well. tests are needed.
may want to move this to the last clause of the if/else. performance tests need to be run for this.
I added it above this elif https://github.com/joy-rosie/pandas/blob/issue-27482-isnull/pandas/core/dtypes/missing.py#L138 because that is causing the error. I will add to _isna_old and also add tests, not sure how to add perfomance tests. |
I have added the tests and the statement to |
|
Hello, I ran the performance tests as shown in the link and this is the final output: . before after ratio
SOME BENCHMARKS HAVE CHANGED SIGNIFICANTLY. Machine information: |
I feel like it is a bit random where the performance has gone down and where its gone up |
Yah, this is a common issue with asv (especially in cases where there are no non-noise changes). The usual solution is to re-run it a couple of times and pick out the changes that are consistent across runs. |
this also needs a whatsnew note, put in 1.0 in Missing bug fix section |
sorry to be a pain but where do i add a whatsnew note? I am also finding it difficult to get any reliable results for the performance tests, I have ran it a few times and there are different results each time Is there anything i can do there? |
Don't worry about it, we were all new here at one point. whatsnew note goes in doc/source/whatsnew/0.25.1.rst
That usually indicates that there isn't any real impact. The non-asv alternative is to use Ipython %timeit directly on the affected function(s) |
Outputs: old:
new:
Please let me know if you think I should run the benchmark on anything else
Will this go into bugfixes or missing? |
Yes. Probably in the "Missing" section |
I added into whatsnew and it seems like there are some things failing but I am not sure why a change in whatsnew could cause these particular failures? |
Restarted the build. That test is randomly failing and we haven't diagnosed the cause yet. |
Is this ready to go now? Let me know if there are any remaining bits. |
LGTM |
…s-dev#27664) * added a check for if obj is instance of type in _isna-new
…e of type in _isna-new (pandas-dev#27664) * added a check for if obj is instance of type in _isna-new
…s-dev#27664) * added a check for if obj is instance of type in _isna-new
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff