-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
Remove arg that is only ever used as NPY_UNSAFE_CASTING #18546
Conversation
…this renders unreachable
Codecov Report
@@ Coverage Diff @@
## master #18546 +/- ##
==========================================
- Coverage 91.35% 91.33% -0.02%
==========================================
Files 164 164
Lines 49802 49802
==========================================
- Hits 45496 45487 -9
- Misses 4306 4315 +9
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@
## master #18546 +/- ##
==========================================
- Coverage 91.35% 91.33% -0.02%
==========================================
Files 164 164
Lines 49802 49802
==========================================
- Hits 45496 45487 -9
- Misses 4306 4315 +9
Continue to review full report at Codecov.
|
can you run an asv set on timeseries and see if anything changes. |
@@ -444,16 +272,6 @@ int parse_iso_8601_datetime(char *str, int len, PANDAS_DATETIMEUNIT unit, | |||
*out_special = 1; | |||
} |
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.
do we need an assertion here that unit is FR_ns?
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.
To double-check, what line are you referring to? The answer is "no" regardless, but the reason why may vary.
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.
parse_iso_8601_datetimetime takes a unit arg, is this vaidated that it is only ever FR_ns
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.
OK. The answer is "no" and the reason is because 1) the unit
arg is no longer used once we've gotten rid of the casting
arg (so it'll be removed in a follow-up) and 2) parse_iso_8601_datetime
is only ever called from src/datetime.pxd and in that one case it is with PANDAS_FR_ns.
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.
ok that's what I figured, ok if you are removing then this is ok.
Starting now. |
|
|
|
There are a few of these; hoping to get them out of the way before doing away with datetime.pxd |
thanks! |
Several functions from src/datetime are only ever called with the casting rule NPY_UNSAFE_CASTING. By getting rid of that dummy argument, the remaining code gets simplified quite a bit.
This PR removes that argument, then removes code that this renders unreachable or unused. It also removes several commented-out functions.
There are a couple of other never-used args; taking these one at a time.