REF: make pytables get_atom_data non-stateful #30074
Conversation
|
I believed Type is covariant so should work for subclasses. Were you seeing an error?
…Sent from my iPhone
On Dec 4, 2019, at 5:27 PM, jbrockmendel ***@***.***> wrote:
cc @WillAyd how do we annotate "this returns a subclass of Col"? Here i've used Type["Col"] but afaik that doesnt include subclasses
You can view, comment on, or merge this pull request online at:
#30074
Commit Summary
REF: make pytables get_atom_data non-stateful
File Changes
M pandas/io/pytables.py (29)
Patch Links:
https://github.com/pandas-dev/pandas/pull/30074.patch
https://github.com/pandas-dev/pandas/pull/30074.diff
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
didnt see an error, was just curious |
|
Yea Type is covariant by default so works for subclasses. TypeVar is invariant so worth knowing that distinction
…Sent from my iPhone
On Dec 4, 2019, at 5:43 PM, jbrockmendel ***@***.***> wrote:
didnt see an error, was just curious
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub, or unsubscribe.
|
|
updated to remove a comment explained the answer to. also made unrelated changes to one comment typo and to re-raise on AssertionError |
| self.dtype = codes.dtype.name | ||
| if values.ndim > 1: | ||
| raise NotImplementedError("only support 1-d categoricals") | ||
|
|
||
| assert self.dtype.startswith("int"), self.dtype | ||
|
|
There was a problem hiding this comment.
Not sure I follow the motivation for this. Seems out of scope.
There was a problem hiding this comment.
making sure that i understand correctly that the codes is always signed-integer
| @@ -1092,6 +1092,8 @@ def remove(self, key: str, where=None, start=None, stop=None): | |||
| except KeyError: | |||
| # the key is not a valid store, re-raising KeyError | |||
| raise | |||
| except AssertionError: | |||
There was a problem hiding this comment.
these are unrelated to most of the PR, but related to the couple-weeks-ago push to get rid of except Exception. In cases where we can't (yet) get rid of except Exception, at least re-raising assertion errors is useful to debugginig
There was a problem hiding this comment.
if this is a sticking point im happy to revert and do this elsewhere
There was a problem hiding this comment.
no its fine, if possible can you add a comment indicating where its raised from
|
pls rebase |
…n-pytables-get_atom_coltype
|
rebased+green |
|
gentle ping, this is a blocker for the payoff ive been promising |
cc @WillAyd how do we annotate "this returns a subclass of Col"? Here i've used
Type["Col"]but afaik that doesnt include subclasses