8000 BUG: Fix #4476 by adding datetime64 and timedelta64 types by gerritholl · Pull Request #5455 · numpy/numpy · GitHub
[go: up one dir, main page]
Skip to content

BUG: Fix #4476 by adding datetime64 and timedelta64 types #5455

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 2 commits into from
Jan 17, 2015
Merged

BUG: Fix #4476 by adding datetime64 and timedelta64 types #5455

merged 2 commits into from
Jan 17, 2015

Conversation

gerritholl
Copy link
Contributor

This commit fixes bug #4476 by adding the codes for the datetime64 and
timedelta64 types to the default_filler dictionary in numpy.ma.core,
used by default_fill_value. Also adapt checking in the
default_fill_value to include code for timedelta64, not only datetime64.

@gerritholl gerritholl mentioned this pull request Jan 15, 2015
Closed
8000
@gerritholl
Copy link
Contributor Author

This is a second attempt to provide the pull request in the correct format, fixing technical git issues that I got wrong in #4477.

@charris
Copy link
Member
charris commented Jan 15, 2015

LGTM. Needs a test, probably in TestFillingValues in numpy/ma/tests/test_core.py. Look at how the other tests are done.

You managed to recover ;) Any problems?

@gerritholl
Copy link
Contributor Author

I added a testcase. Should I merge the commit for fixing the problem and the commit for adding the testcase to a single commit?

@gerritholl
Copy link
Contributor Author

I did notice one small thing; the representation for numpy.datetime64('NaT') does not indicate the timecode, but numpy.ma.testutils.assert_equal does (fortunately) pay attention to this. Consequently, if the timecode is wrong, the testing system will report:

Items are not equal:
 ACTUAL: numpy.datetime64('NaT')
 DESIRED: numpy.datetime64('NaT')

which is potentially going to confuse somebody in the future. But that's a different issue (should datetime64.__repr__ show the time code?).

@gerritholl
Copy link
Contributor Author

It does appear I have messed up again. I will try again to fix it.

@charris
Copy link
Member
charris commented Jan 15, 2015

Just say no to merge, use rebase ;) That said, the only reason to rebase is 1) Clean up history, rewrite commit messages, etc. 2) PR is not mergable into master.

No need to squash the two commits, but the second should begin TST:.

@gerritholl gerritholl force-pushed the masked_structured_datetime64 branch from 655d2d6 to 077d33a Compare January 15, 2015 20:19
@charris
Copy link
Member
charris commented Jan 15, 2015

It's easy to recover from a bad merge, just git reset --hard ORIG_HEAD.

@gerritholl gerritholl force- 8000 pushed the masked_structured_datetime64 branch from 077d33a to 1bf5d56 Compare January 15, 2015 20:20
@gerritholl gerritholl force-pushed the masked_structured_datetime64 branch from 1bf5d56 to 077d33a Compare January 15, 2015 20:22
Gerrit Holl added 2 commits January 15, 2015 15:32
BUG: Fix numpy#4476 by adding datetime64 and timedelta64 types
4bfcf48 
This commit fixes bug numpy#4476 by adding the codes for the datetime64 and
timedelta64 types to the `default_filler` dictionary in numpy.ma.core,
used by `default_fill_value`.  Also adapt checking in the
`default_fill_value` to include code for timedelta64, not only datetime64.
TST: Add testcase for the fix to bug numpy#4476.
00ee332 
Add a testcase `test_fillvalue_datetime_timedelta` to class
`TestFillingValues` for the fix to bug numpy#4476.  See commit
216fd17 and pull request numpy#5455.
@gerritholl gerritholl force-pushed the masked_structured_datetime64 branch from 077d33a to 00ee332 Compare January 15, 2015 20:34
@charris
Copy link
Member
charris commented Jan 15, 2015

Hmm, not sure how you got there, branch looks OK git log --graph. You can use git commit --amend to change the author. The lines of the commit message should also be at most 72 characters long.

@gerritholl
Copy link
Contributor Author 8000

Ok. I hope it's all OK now, I undid everything and made it as two new commits.

@gerritholl
Copy link
Contributor Author

Do I need to do anything to keep this "up to date" as this pull request stands while the upstream/numpy main branch moves on?

@charris
Copy link
Member
charris commented Jan 16, 2015

Nope, not unless the merge button turns grey and the PR cannot be merged. That is unlikely unless someone edits the same files in almost the same places.

charris added a commit that referenced this pull request Jan 17, 2015
@charris
Merge pull request #5455 from gerritholl/masked_structured_datetime64
1d87101 
BUG: Fix #4476 by adding datetime64 and timedelta64 types
@charris charris merged commit 1d87101 into numpy:master Jan 17, 2015
@charris
Copy link
Member
charris commented Jan 17, 2015

Merged, thanks @gerritholl .

juliantaylor pushed a commit to juliantaylor/numpy that referenced this pull request Jan 25, 2015
@juliantaylor
TST: Add testcase for the fix to bug numpy#4476.
53b19c5 
Add a testcase `test_fillvalue_datetime_timedelta` to class
`TestFillingValues` for the fix to bug numpy#4476.  See commit
216fd17 and pull request numpy#5455.
@jjhelmus jjhelmus mentioned this pull request Oct 21, 2015
Closed
@gerritholl gerritholl deleted the masked_structured_datetime64 branch May 20, 2020 15:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@gerritholl @charris
0