REF: use new pytables helper methods to de-duplicate _convert_index #30144
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jbrockmendel
added
Refactor
jreback
requested changes
Dec 8, 2019
| @@ -2318,6 +2309,9 @@ def get_atom_coltype(cls, kind: str) -> Type["Col"]: | |||
| if kind.startswith("uint"): | |||
| 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
jreback
approved these changes
Dec 10, 2019
|
looks good. if you can do follows as indicated above when you have a chance. |
proost
pushed a commit
to proost/pandas
that referenced
this pull request
Dec 19, 2019
proost
pushed a commit
to proost/pandas
that referenced
this pull request
Dec 19, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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.