Skip to content
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

BUG: Check whether the datatype is of Datetime before typecasting as usual. #19722

Closed
wants to merge 9 commits into from

Conversation

Dinkarkumar
Copy link

@Dinkarkumar Dinkarkumar commented Aug 21, 2021

Closes #19267, np.allclose fails with dtype datetime64

This is my first contribution, Seeking guidance and support from such huge community.

@charris charris changed the title Added condition to check whether the datatype is of Datetime, if Not then proceed else typecast as ususal. BUG: Check whether the datatype is of Datetime before typecasting as usual. Aug 23, 2021
README.md Outdated Show resolved Hide resolved
@mattip
Copy link
Member

mattip commented Aug 26, 2021

Please add a test that fails before, passes after the change.

@Dinkarkumar
Copy link
Author

Reproducing code example:
Earlier the following code snippet used to give TypeError:-
import numpy as np
a = np.zeros(1, dtype='datetime64[ns]')
np.allclose(a, a)

Now after code changes the typecasting would not be possible and the np.allclose(a,a) would run for even dtype = 'datetime64[ns]'
@mattip , i am not yet convinced of why do you ask me to revert the chnanges. Let's discuss and resolve the issue.

@mattip
Copy link
Member

mattip commented Aug 29, 2021

I asked only that you revert the changes to README.md. You still did not add a test, the place to add it would be numpy/core/tests/test_numeric.py.

README.md Show resolved Hide resolved
@Dinkarkumar
Copy link
Author

Sorry for such immature execution of my task. I am totally new to this environment. Hope to learn and seeking guidance from you all.

@mattip
Copy link
Member

mattip commented Aug 30, 2021

I think you might want to back out all the changesets other than abe9ff0 and then add the test in a second changeset. Your PR should only consist of

  • a one-line change to numpy/core/numeric.py
  • a test added/modified in numpy/core/tests/test_numeric.py, which should also be at the most 10 lines.

@seberg
Copy link
Member

seberg commented Apr 28, 2023

I am going to close this. The fix is likely right, but its a mess unfortunately and old. It also needs a rebase anyway, creating a new PR from scratch seems just as well (which anyone is very welcome to do!).

@seberg seberg closed this Apr 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Development

Successfully merging this pull request may close these issues.

np.allclose fails with dtype datetime64
5 participants