Assorted _libs cleanups #22235
Assorted _libs cleanups #22235
Conversation
|
|
||
|
|
||
| cdef int min_value(int left, int right) nogil: | ||
| if left < right: |
There was a problem hiding this comment.
grr how many times do we have this i the code base :)
There was a problem hiding this comment.
We could probably use np_datetime.cmp_scalar here, would need to cast from int to int64_t
There was a problem hiding this comment.
no it’s done - was just generally commenting
| int col = max_value(from_index, to_index) | ||
| # row or col < 6 means frequency strictly lower than Daily, which | ||
| # do not use daytime_conversion_factors | ||
| if row < 6: |
There was a problem hiding this comment.
can u use the enumerated here?
There was a problem hiding this comment.
huh? This is just moving the existing C code into cython.
There was a problem hiding this comment.
I get it, but the 6 is pretty opaque
| int col = max_value(from_index, to_index) | ||
| # row or col < 6 means frequency strictly lower than Daily, which | ||
| # do not use daytime_conversion_factors | ||
| if row < 6: |
There was a problem hiding this comment.
I get it, but the 6 is pretty opaque
pandas/_libs/src/skiplist.h
Outdated
| #define PANDAS_INLINE | ||
| #endif | ||
| #endif | ||
| #include "helper.h" |
There was a problem hiding this comment.
can we rename helper to something more informative
| @@ -1103,7 +1102,7 @@ class Timestamp(_Timestamp): | |||
|
|
|||
|
|
|||
| # Add the min and max fields at the class level | |||
| cdef int64_t _NS_UPPER_BOUND = INT64_MAX | |||
There was a problem hiding this comment.
future TODO, we look up these limits all over the place. maybe make a limits.pyx somewhere
Codecov Report
@@ Coverage Diff @@
## master #22235 +/- ##
=======================================
Coverage 92.08% 92.08%
=======================================
Files 169 169
Lines 50704 50704
=======================================
Hits 46691 46691
Misses 4013 4013
Continue to review full report at Codecov.
|
|
lgtm. merge on green. |
|
@pandas-dev/pandas-core do we have any windows users who can help troubleshoot appveyor-only build failures? |
|
@jbrockmendel : I wasn't able to reproduce the AppVeyor failure unfortunately. That being said, very hard to believe that the removal of the profiler magic comment is the cause of the issue. But I have to ask: what happens if you put the magic comment back? |
Darn, thanks for trying.
Its worth a try, but I expect the issue isn't with resolution.pyx but in one of its dependencies. |
|
I can reproduce - likely dependent on the 2.7 toolchain - will see what I can figure out |
|
Made a pull on your PR - long story short, it appears you were depending in a couple spots on an implicit include of |
some typedefs for MSVC 2008
|
thanks @chris-b1 ok lgtm, merge on green (is the appveyor tests sufficient for this?) |
|
@jreback : The AppVeyor test should be sufficient since it was a build issue. Also, I would have been much more concerned had it occurred on Python 3.x (instead of 2.x in this case). |
|
Thanks @jbrockmendel ! |
Modernize for-loop syntax
Modernize string formatting
Use memoryviews in a few places
Docstrings, change a couple of funcs in tslibs.parsing to cdef
Follow-up cleanup in tslibs.period
Move some unneeded stuff out of tslibs.util to wrap up the self-contained goal.