TYP: stubs for tslibs #40433

jbrockmendel · 2021-03-14T18:17:06Z

closes #xxxx
tests added / passed
Ensure all linting tests pass, see here for how to run them
whatsnew entry

MarcoGorelli · 2021-03-14T20:41:37Z

pandas/_libs/tslibs/ccalendar.pyi

@@ -0,0 +1,13 @@
+
+DAYS: list[str]


how does list[str] work in Python < 3.9? (asking because I don't know)

i decided to try this out bc i think ive seen @simonjayhawkins using this pattern using it recently

how does list[str] work in Python < 3.9? (asking because I don't know)

pyi files don't go through the Python interpreter. so have the advantage of being able to use the latest typing features understood by mypy/pyre/pyright/pytype. list[str] is PEP 585 compliant.

side note: we are in the process of attempting to inline the microsoft pyright stubs. inline types not only need to satisfy the python interpreter supported (i.e. python 3.7 without typing extensions) but the code in the annotated functions also needs to be consistent with the in-line function annotations.

so (hopefully without sounding too negative) I imagine that not only will that take a while but may also not even be possible in some cases.

For type checking user code against the public api, I expect using stubs (eg. bundled with pyright or 3rd party such as https://github.com/predictive-analytics-lab/data-science-types (development ceased 16 Feb 2021)) will be the better option for the foreseeable future.

However, internal consistency/robustness is achieved though adding inline types and checking with mypy (which has excellent features that support the gradual typing process.)

we do yet have a good way of generating type stubs from cython files, so in order to have types available for the compiled code, we probably need to manually curate these. So being able to use the pyright stubs here would be beneficial.

@jbrockmendel Are these stubs manually curated or forked from pyright? I believe @Dr-Irv is coordinating between pandas and pyright and may have some input on this.

Are these stubs manually curated or forked from pyright?

Manually curated

I think doing this is OK, as adding stubs may reveal problems in the python code that may need to be fixed. So doing this before pulling in the pyright stubs could be advantageous and make the migration smoother.

@jbrockmendel Are these stubs manually curated or forked from pyright? I believe @Dr-Irv is coordinating between pandas and pyright and may have some input on this.

I wouldn't say I'm "coordinating", but the work that has been done by the Microsoft team provides a good starting point (and possible reference) for pandas type stubs. See https://github.com/microsoft/python-type-stubs/tree/main/pandas

simonjayhawkins

Thanks @jbrockmendel generally lgtm.

simonjayhawkins · 2021-03-15T14:43:35Z

pandas/_libs/tslibs/strptime.pyi

+
+def array_strptime(
+    values: np.ndarray,  # np.ndarray[object]
+    fmt: Optional[str],


str | None and then don't need the import.

simonjayhawkins · 2021-03-15T14:45:21Z

pandas/_libs/tslibs/strptime.pyi

+def array_strptime(
+    values: np.ndarray,  # np.ndarray[object]
+    fmt: Optional[str],
+    exact: bool = True,


don't add the defaults. just ... less to keep in sync

simonjayhawkins · 2021-03-15T14:45:32Z

pandas/_libs/tslibs/strptime.pyi

+    values: np.ndarray,  # np.ndarray[object]
+    fmt: Optional[str],
+    exact: bool = True,
+    errors: str = "raise"


simonjayhawkins · 2021-03-15T14:47:30Z

pandas/_libs/tslibs/timezones.pyi

+    Optional,
+    Union,


could lose these imports using the newer syntax

simonjayhawkins · 2021-03-15T14:51:04Z

pandas/_libs/tslibs/timezones.pyi

+import numpy as np
+
+# imported from dateutil.tz
+dateutil_gettz: Callable[[str], tzinfo]


maybe we could import from dateutil.tz directly in the stub.

simonjayhawkins · 2021-03-15T14:54:13Z

pandas/_libs/tslibs/timezones.pyx

@@ -80,6 +81,8 @@ cpdef inline object get_timezone(tzinfo tz):
    the tz name. It needs to be a string so that we can serialize it with
    UJSON/pytables. maybe_get_tz (below) is the inverse of this process.
    """
+    if tz is None:


you've defined def get_timezone(tz: tzinfo) -> Union[tzinfo, str]: .... tz shouldn't be None? (from python code anyway)

with cython semantics tzinfo tz doesn't exclude None, so we have to do it ourselves

simonjayhawkins · 2021-03-15T14:57:11Z

pandas/_libs/tslibs/timezones.pyx

@@ -364,6 +367,8 @@ cpdef bint tz_compare(tzinfo start, tzinfo end):
    elif is_utc(end):
        # Ensure we don't treat tzlocal as equal to UTC when running in UTC
        return False
+    elif start is None or end is None:


probably same answer as above. can start or end be None?

simonjayhawkins · 2021-03-15T14:57:48Z

pandas/_libs/tslibs/tzconversion.pyi

+def tz_localize_to_utc(
+    vals: np.ndarray,  # np.ndarray[np.int64]
+    tz: Optional[tzinfo],
+    ambiguous: Optional[Union[str, bool, Iterable[bool]]] = None,


... for default

simonjayhawkins · 2021-03-15T14:57:56Z

pandas/_libs/tslibs/tzconversion.pyi

+    vals: np.ndarray,  # np.ndarray[np.int64]
+    tz: Optional[tzinfo],
+    ambiguous: Optional[Union[str, bool, Iterable[bool]]] = None,
+    nonexistent: Optional[Union[str, timedelta, np.timedelta64]] = None,


simonjayhawkins · 2021-03-15T14:58:14Z

pandas/_libs/tslibs/tzconversion.pyi

+    Optional,
+    Union,


same comment as before

mind if i apply these comments in the next pass? already have a branch on deck

simonjayhawkins · 2021-03-15T15:22:41Z

in setup.cfg we have ignore_missing_imports = True. This is probably the next strictness flag we should consider changing.

we this PR have 393 errors, 162 of these are from pandas._libs

on master we have 407 errors (176 pandas._libs)

so a good start.

simonjayhawkins

mind if i apply these comments in the next pass? already have a branch on deck

sure. another comment for followup and an observation.

simonjayhawkins · 2021-03-15T16:46:24Z

pandas/_libs/tslibs/tzconversion.pyi

+    tzinfo,
+)
+from typing import (
+    Iterable,


from https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#stub-file-coding-style

in Python 3 stubs, import collections (Mapping, Iterable, etc.) from collections.abc instead of typing;

simonjayhawkins · 2021-03-15T16:53:27Z

pandas/_libs/tslibs/timezones.pyi

+
+def maybe_get_tz(tz: Optional[Union[str, int, np.int64, tzinfo]]) -> Optional[tzinfo]: ...
+
+def get_timezone(tz: tzinfo) -> Union[tzinfo, str]: ...


again from https://github.com/python/typeshed/blob/master/CONTRIBUTING.md#stub-file-coding-style

avoid Union return types: python/mypy#1693

although not sure this is possible without a refactor.

yah id like to change this behavior too, but should be separate from typing

WillAyd

lgtm - great to see these types coming in

simonjayhawkins · 2021-03-16T13:18:37Z

Thanks @jbrockmendel

jbrockmendel added 2 commits March 14, 2021 11:15

TYP: stubs for tslibs

889e65f

revert strptime

229a13a

MarcoGorelli reviewed Mar 14, 2021

View reviewed changes

simonjayhawkins added the Typing type annotations, mypy/pyright type checking label Mar 15, 2021

simonjayhawkins reviewed Mar 15, 2021

View reviewed changes

simonjayhawkins approved these changes Mar 15, 2021

View reviewed changes

simonjayhawkins added this to the 1.3 milestone Mar 15, 2021

WillAyd approved these changes Mar 15, 2021

View reviewed changes

simonjayhawkins merged commit 940695b into pandas-dev:master Mar 16, 2021

jbrockmendel deleted the typ-libs-2 branch March 16, 2021 14:26

JulianWgs pushed a commit to JulianWgs/pandas that referenced this pull request Jul 3, 2021

TYP: stubs for tslibs (pandas-dev#40433)

1de556b

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TYP: stubs for tslibs #40433

TYP: stubs for tslibs #40433

jbrockmendel commented Mar 14, 2021

MarcoGorelli Mar 14, 2021

jbrockmendel Mar 14, 2021

simonjayhawkins Mar 15, 2021

jbrockmendel Mar 15, 2021

simonjayhawkins Mar 15, 2021

Dr-Irv Mar 15, 2021

simonjayhawkins left a comment

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

jbrockmendel Mar 15, 2021

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

jbrockmendel Mar 15, 2021

simonjayhawkins commented Mar 15, 2021

simonjayhawkins left a comment

simonjayhawkins Mar 15, 2021

simonjayhawkins Mar 15, 2021

jbrockmendel Mar 15, 2021

WillAyd left a comment

simonjayhawkins commented Mar 16, 2021


		def maybe_get_tz(tz: Optional[Union[str, int, np.int64, tzinfo]]) -> Optional[tzinfo]: ...

		def get_timezone(tz: tzinfo) -> Union[tzinfo, str]: ...

TYP: stubs for tslibs #40433

TYP: stubs for tslibs #40433

Conversation

jbrockmendel commented Mar 14, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins commented Mar 15, 2021

simonjayhawkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

simonjayhawkins commented Mar 16, 2021