ENH: Export (a subset of?) pandas._typing for type checking #55231
Comments
caneff
added
Enhancement
Needs Triage
|
A subset of _typing exposed in #48577 |
|
Ok so any blocker to me exposing some of the literals in that same place
then? Like MergeHow and CorrelationMethod are the two we have run into the
most at Google when trying to upgrade to pandas 2 but I'm sure I can find
others looking through that file that make sense in the same way.
…On Fri, Sep 22, 2023, 8:10 AM Torsten Wörtwein ***@***.***> wrote:
A subset of _typing exposed in #48577
<#48577>
—
Reply to this email directly, view it on GitHub
<#55231 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABG4SH6X34LA4TPXXBBUE5TX3V53HANCNFSM6AAAAAA5B3KDFE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I think We also have pandas-stubs (but you can obviously not import from there). |
|
Not sure why you can't just use Alternatively, we could have a There is also the issue of creating the documentation for things like So, this isn't just an issue of whether we start exposing these literals, it's also a documentation issue. |
|
`pandas._typing` isn't a solution because it is protected and not
expected to be imported from other packages. It is never a good idea to
break that agreement, especially for functionality this mundane (type
checking of inputs to already public methods).
If we are exposing types through arguments of public methods, we should be
able to access those types in a way that isn't through whatever
`pandas.core` file happened to import them, or by `typing.get_type_hints`
or the like.
I'm fine adding documentation mentioned as well if that is the only
objection.
…On Fri, Sep 22, 2023, 9:53 AM Irv Lustig ***@***.***> wrote:
Not sure why you can't just use from pandas._typing import MergeHow to
address your current issue.
Alternatively, we could have a pandas/typing.py for the ones we are
willing to export (or maybe put all of those in pandas.api.typing) and
keep pandas/_typing.py for internal usage.
There is also the issue of creating the documentation for things like
MergeHow. If we put them in a public place (whether pandas.typing or
pandas.api.typing, then they would need to be documented, so that
changing the names of the types (if that should ever happen) would be known
to the public.
So, this isn't just an issue of whether we start exposing these literals,
it's also a documentation issue.
—
Reply to this email directly, view it on GitHub
<#55231 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABG4SHY3UMFXHWIKKILIMS3X3WJ4NANCNFSM6AAAAAA5B3KDFE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
I think But the main concern is with evolution of type hints alongside the library. How do we go about changing a name or removing a type-hint without breaking user code? If type-hints were strings, then I don't think we'd be at risk. For example: is valid Python. But they currently aren't by default and without the future import this code fails. I don't believe we have any way to signal to a user that a type-hint is deprecated or going to change. |
|
I agree with that sentiment but presumably it is equivalent even if we
don't export these. If you are going to widen the literals to more values,
then some code that had actually copied the literal definition of or
someone importing from _typing is the same. If you are reducing it to fewer
values, then they will break the same way and can be deprecated by
detecting usage of the value basically. Not exposing them saves a
deprecation only on the case of a rename, to the expense of no one else
being able to access it without violating the protectedness of _typing.
I agree with your point but it is a different one from my original question
…On Fri, Sep 22, 2023, 8:28 PM Richard Shadrach ***@***.***> wrote:
I think pandas.api.typing would make sense for this, and find having
pandas.typing alongside pandas.api.typing a bit confusing.
But the main concern is with evolution of type hints alongside the
library. How do we go about changing a name or removing a type-hint without
breaking user code? If type-hints were strings, then I don't think we'd be
at risk. For example:
from __future__ import annotations
def foo(x) -> bar:
return x + 1
foo(1)
is valid Python. But they currently aren't by default and without the
future import this code fails. I don't believe we have any way to signal to
a user that a type-hint is deprecated or going to change.
—
Reply to this email directly, view it on GitHub
<#55231 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABG4SH2IREQIVCYWAXFQQW3X3YULXANCNFSM6AAAAAA5B3KDFE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
|
Two thoughts:
|
|
1. That's acceptable to me.
2. At Google we haven't adopted stubs yet (but it so on my list). We depend
on the bare types ATM and given our size of codebase it will be a slow
transition.
Do pandas-stubs not just import from typing anyway? Isn't that how we keep
things in sync?
…On Sat, Sep 23, 2023, 10:56 AM Torsten Wörtwein ***@***.***> wrote:
Two thoughts:
- We could separate the stable panas.api.typing and create a new
pandas.api.typing.aliases that is documented to be public but NOT
stable (names might change without warnings between releases and the values
too) - typing pandas is already slow, I would like to avoid more friction
(adding deprecation/future warnings for typing changes)
- Most people probably do not use the panda's internal annotations
(only bare pyright people or people who specifically add a py.typed),
pylance and presumably most mypy people use pandas-stubs: if we expose type
aliases, we need to ensure that they are in sync between pandas and
pandas-stubs!
—
Reply to this email directly, view it on GitHub
<#55231 (comment)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/ABG4SHYNLHFP7ISSUUSK47DX332BJANCNFSM6AAAAAA5B3KDFE>
.
You are receiving this because you authored the thread.Message ID:
***@***.***>
|
I think you will find that using
|
|
Yes I agree. But with the scale of our codebase makes it daunting to say
the least. Any bit of new typing results in a lot of new typing violations
to be handled. Any suggestions for how to maybe introduce them piecewise?
Also how would the stubs help with my current issue though? Do the stubs
intentionally export MergeHow anywhere?
|
With
I think you could do something like this: from typing import TYPE_CHECKING
if TYPE_CHECKING:
from pandas._typing import MergeHow
def foo(df: pd.DataFrame, ..., how: "MergeHow"):
...
pd.merge(...., how=how) |
lithomas1
removed
the
Needs Triage
|
@WillAyd Back in 2019, you added this text in #27050 (slightly edited since then) that now appears at https://pandas.pydata.org/pandas-docs/stable/development/contributing_codebase.html#pandas-specific-types that says: "Commonly used types specific to pandas will appear in pandas._typing and you should use these where applicable. This module is private for now but ultimately this should be exposed to third party libraries who want to implement type checking against pandas."
Ideally, we would move more of the "public" types into a module and document them, and have a correspondence between both One quick option is we don't worry about the documentation, and just make In any case, I think we need to now handle the sentence you wrote back then: " This module is private for now but ultimately this should be exposed to third party libraries who want to implement type checking against pandas." Thoughts? Who else should we include in this conversation? (Adding @rhshadrach to get his thoughts) |
|
What are the objects from
Then any future changes to the objects in Would we be aligned to "usage of pandas type-hinting objects is subject to breakage in even minor versions of pandas". Would users? I'm also negative on having both |
|
Reading over the linked pyright issue, people importing from a private module are breaking Python conventions. They are free to do so at their own peril, but it is not onerous to add a That said, I'm not necessarily adverse to exposing some subset in a public location, but the real-world utility should be demonstrated first. |
Presumably this statement pre-dates pandas-stubs. I guess this was in the early days of pandas typing journey when how best to help users type check their pandas code had not been decided?
Agreed. I think that now we should agree that
I would not be adverse to renaming this if to avoid confusion. The classes here are public. They are mostly returned from pandas operations and not directly constructed. So effectively we cannot change these classes without breaking backwards compatibility. The ARE part of the API, but were not included in the documented API section because users are not expected to construct these. The reason that they were put in |
of course, the simplest solution may be to document these properly and just include them in |
Yes, it is from 2019 and
The issue that we now have is that people are doing imports from
It's also because they appear in the docs. E.g., we document that So I actually think that whatever "types" we want to make public should be in |
maybe I'm misunderstanding. This sounds like the tail wagging the dog?
Again I must be misunderstanding. It's not the "types" that we WANT to make public. It's anything that our public API returns that defacto is also public? |
|
Sorry for missing the original ping. It has been a while and a lot has changed in the landscape, but I think I agree with @simonjayhawkins as to the original intent of that private module, i.e. the module itself was not necessarily intended to be public in the way we normally expose objects, but rather the types included describe the API that we have I don't have a strong preference on how to proceed as I haven't been involved in types in quite some time. I am happy to defer to whatever you think is best |
At least for right now, the only way I can see to determine which types to expose are the ones that are used in the tests of
It's anything that our public API returns, PLUS anything that is a type of a method/function argument. For example, in CompressionOptions = Optional[
Union[Literal["infer", "gzip", "bz2", "zip", "xz", "zstd", "tar"], CompressionDict]
]and then a method like def compress_my_df(df: pd.DataFrame, copt: CompressionOptions):
return df.to_json(..., compression = copt) |
I assume aliases used by end users type checking their code would be taken from pandas-stubs and NOT from pandas? IIRC we discussed ways of generally keeping these in sync but the types in pandas._typing and the aliases defined there are for typing checking the pandas codebase ONLY (i.e. internally for robustness) and end users should NOT be using these directly from pandas? |
As a user, you'd then have to surround any So it's better if whatever is imported by a user is both in
Well, yes, end users should NOT be doing So the idea (which is the original intent of this issue) is that we pick a subset of |
This seems to be a deviation from our original motivation of creating the stubs library in order to decouple the typing for end users from the codebase internal checks?
-1 We should retain the freedom to improve and enhance the internal codebase type checking without the constraints of breaking changes and deprecations? |
I would say our original motivation was to provide typing support for users of the API above what is handled in the codebase. But I have maintained a policy that if you import something in your code that is documented, it should work at runtime for users. This works fine if a user doesn't import something from
Absolutely. But I think there are types in |
|
I appreciate the in-depth conversation on how users are relying on types from Of course, we need to be cautious about committing to a public API that might constrain our ability to evolve internal type checking. Clear documentation and a careful inventory of the types that are safe to expose would go a long way toward balancing stability with internal flexibility. Overall, this seems like a promising direction to improve type checking support for users while keeping our internal evolution agile. IIUC @twoertwein and @rhshadrach also seem to have similar reservations to myself. @Dr-Irv If the proposed solution was labeled as experimental, allowing us to make changes at any time, would that be acceptable to you and the others? |
Yes, I'd be fine with that. |
|
I think I prefer @twoertwein suggestion in #55231 (comment) of using |
Seems reasonable to me. |
… check to enforce imports from submodules whose names begin with an underscore. This change was disruptive to users of the popular `pandas` library because it exports a submodule named `_typing`. This was originally intended to be experimental or private, but it was never fully addressed. The pandas maintainers will work to fix this issue (pandas-dev/pandas#55231) over the next year. I'm going to back out this change to pyright in the meantime.
… check to enforce imports from submodules whose names begin with an underscore. This change was disruptive to users of the popular `pandas` library because it exports a submodule named `_typing`. This was originally intended to be experimental or private, but it was never fully addressed. The pandas maintainers will work to fix this issue (pandas-dev/pandas#55231) over the next year. I'm going to back out this change to pyright in the meantime. (#10322)
|
Is there any common practice for naming additional typing-only modules included to the stub? Met the similar To give some background - it's stubs for Blender Python API. Blender's Python API for the most part lives in C++ and there are lots of enums that API is using but they're not really exposed to Python API as such. |
As I understand it, the issue is whether you are importing a private-only module (e.g., In the case of You can create a stub-only module for typing, as long as only the stubs import that module. Runtime code would fail if you had a stubs-only module. |
|
@rhshadrach Here is a proposal of types to initially put in
I based this on looking at the tests we do in I still have to look at some other aliases that are used as arguments to various methods to see what else could be added (e.g., Comments and criticism welcome! |
I started to look at this and am not sure what to do. In the OP, he wrote:
Let's consider a function like def read_parquet(
path: FilePath | ReadBuffer[bytes],
engine: ParquetEngine = ...,
columns: list[str] | None = ...,
storage_options: StorageOptions = ...,
**kwargs: Any,
) -> DataFrame: ...Here are the relevant types in from os import PathLike
FilePath: TypeAlias = str | PathLike[str]
AnyStr_cov = TypeVar("AnyStr_cov", str, bytes, covariant=True)
class BaseBuffer(Protocol):
@property
def mode(self) -> str: ...
def seek(self, offset: int, whence: int = ..., /) -> int: ...
def seekable(self) -> bool: ...
def tell(self) -> int: ...
class ReadBuffer(BaseBuffer, Protocol[AnyStr_cov]):
def read(self, n: int = ..., /) -> AnyStr_cov: ...
ParquetEngine: TypeAlias = Literal["auto", "pyarrow", "fastparquet"]
StorageOptions: TypeAlias = dict[str, Any] | NoneIf the goal is to allow someone to write a cover function for On the other hand, maybe we should be more selective, and only include types like |
|
I think we should not expose an alias like
We should never expose |
I don't agree. If you call And for
Yes, I agree, but maybe I should just go through all of |
|
To be sure, I'd be fine with something like |
Feature Type
Adding new functionality to pandas
Changing existing functionality in pandas
Removing existing functionality in pandas
Problem Description
There are public functions and methods whose arguments are types that are not actually exported. This makes it hard to propogate those types to other functions that then call the pandas ones.
For instance,
mergehas ahowargument that has a type_typing.MergeHow = Literal['cross', 'inner', 'left', 'outer', 'right'], but since_typingis protected, there is no good way to take it as an argument and instead I have to sayFor my type checker to be OK with it. This is both annoyingly verbose and fragile to updates of Pandas
Feature Description
Add a
typingmodule that exposes a (possible subset) of_typing.I say possible subset because from looking at the
_typingmodule there are clearly types that are internal usage only and I'm guessing we don't want to have them public so that they can be changed easier.I would propose the subset be all types that are used as arguments of public functions and methods.
This way my function above could have;
and have everything work.
Alternative Solutions
Technically these types are "available" when imported by other modules, so you can access
MergeHowviapandas.core.reshape.merge.MergeHoworpandas.core.frame.MergeHowbut those are just imports from_typingimported to be used by those modules themselves, not something users should rely on.Other alternatives
A) Split the public ones out of
_typingintotyping, couldfrom typing import *in_typingif we don't want to rewrite everywhere the newly public types are used.B) Just make all of
typingpublic. As someone who is not heavy into Pandas internals I have no strong opinion here but my guess is that there are internal types that we don't want public.Additional Context
I'm more than willing to take this PR myself I just want feedback about whether this would be accepted.
The text was updated successfully, but these errors were encountered: