REF: use new pytables helper methods to de-duplicate _convert_index #30144
Conversation
jbrockmendel
added
Refactor
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
looks good about the assertions
| k4 = kind[4:] | ||
| col_name = f"UInt{k4}Col" | ||
| elif kind.startswith("period"): | ||
| # we store as integer |
There was a problem hiding this comment.
we may have very little coverage for PeriodIndex FYI
| # For datetime64tz we need to drop the TZ in tests TODO: why? | ||
| dtype_name = data.dtype.name.split("[")[0] | ||
|
|
||
| if data.dtype.kind in ["m", "M"]: |
There was a problem hiding this comment.
use needs_i8_conversion (followon ok)
| # TODO: we used to reshape for the dt64tz case, but no longer | ||
| # doing that doesnt seem to break anything. why? | ||
|
|
||
| elif isinstance(data, PeriodIndex): |
There was a problem hiding this comment.
can likely use extract_array here
There was a problem hiding this comment.
that would give PeriodArray, we need to get i8 back here
There was a problem hiding this comment.
right once you have the arrays, then you can simply .view('i8') and you are done (pretty generically)
There was a problem hiding this comment.
ok i see what you're saying now. will do on next pass (along with needs_i8 mentioned above)
…f-pytables-convert_index
|
looks good. if you can do follows as indicated above when you have a chance. |
2 participants
The assertions in here are intended to be temporary. Once reviewers are convinced this is accurate, then the assertions can be removed and a bunch of these blocks can be condensed down to share code.
The new helper func _get_data_and_dtype_name is also going to be used later to pre-empt the need for the state-altering
get_datamethod.