CLN: Clean DirNameMixin._deprecated #28957
Conversation
topper-123
changed the title
topper-123
force-pushed
the
clean_DirNameMixin
branch
from
e6f67dc
to
37d02e7
Compare
pandas/core/base.py
Outdated
| @@ -50,6 +50,8 @@ | |||
| class PandasObject(DirNamesMixin): | |||
| """baseclass for various pandas objects""" | |||
|
|
|||
| _deprecations = DirNamesMixin._deprecations | frozenset() # type: FrozenSet[str] | |||
There was a problem hiding this comment.
Is the rhs of the union doing anything here? I assume you are just stubbing out for future readers but in that case the annotation is unnecessary.
Probably cleanest to just leave as is for now and can be overridden in future as required
There was a problem hiding this comment.
Yeah, it's for the readers. I don't think this pattern of copying frozensets is seen very much, so thought it helps a bit to just have it stubbed out.
| "flags", | ||
| "strides", | ||
| ] | ||
| ) # type: FrozenSet[str] |
There was a problem hiding this comment.
You might not need this annotation; I think mypy should be able to infer
There was a problem hiding this comment.
Tried again, and mypy complains without the annotation. My guess is that it cannot be sure that the added set should only contain strings.
| PandasObject._deprecations | ||
| | IndexOpsMixin._deprecations | ||
| | frozenset(["asobject", "contains", "dtype_str", "get_values", "set_value"]) | ||
| ) # type: FrozenSet[str] |
There was a problem hiding this comment.
I added the type hints after complaints from mypy. I agree it's strange it doesn't infer and I'll see if something can be done to make it infer.
There was a problem hiding this comment.
I think should be able to infer as long as you construct with values. Empty construction complains because it is not known then what types of value the object should hold.
Not sure if FrozenSet would act differently but that’s how builtins work
There was a problem hiding this comment.
Same here. A frozenset that only contains strings, could conceivably have non-strings, and I think that's what mypy complains about.
topper-123
force-pushed
the
clean_DirNameMixin
branch
from
37d02e7
to
f62fdf1
Compare
|
Thanks @topper-123 |
This moves the content of
DirNameMixin._deprecationsto more appropriate locations, typicallyIndexOpsMixIn._deprecations, as that is a common subclass ofIndexandSeries. The names inDirNameMixin._deprecationsbelonged to those two classes, so having the deprecated names located all the way up inDirNameMixinmade them be available in all classes that subclassDirNameMixin, which was unfortunate.By having
DirNameMixin._deprecationsstart with an empty set, it will be easier to useDirNameMixinandPandasObjectwhere/when needed, without inheriting undesired names in_deprecated.This PR also moves
"tolist"toIndexOpsMixIn._deprecations, becausetolistis defined in that class.