-
-
Notifications
You must be signed in to change notification settings - Fork 7.9k
[Doc]: Most functions in axes.Axes accept datetime objects as well as floats, docstrings should reflect that #25676
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Comments
Thanks for reporting this and #25677! Note that for Matplotlib 3.8 we will provide our own stubs, so I'll also add the types label to those issues (and most likely pylance will not update their stubs). |
Due to the way the units handling works it is run-time configurable the exact types that can be passed in. |
By default Matplotlib also accepts lists of strings:
and as @tacaswell points out, it can accept anything so long as there is a converter set up for it: https://matplotlib.org/stable/api/units_api.html#module-matplotlib.units I don't know how we convey all of that in the docstring of each and every plotting method that has units. OTOH, it wouldn't be a terrible idea for us to try, particularly in the case of methods that require deltas (eg errorbar with timedelta + datetime) that require extra explanation. One issue with going through this effort is we are trying to overhaul data (and hence units) handling. |
Should we let the pylance folks about this? |
I'm on the Pylance team. This is great news! When is 3.8 expected to ship? |
We aim for a ~6mo release cadence so probably July/August. I am going to advocate for an RC by scipy, but we have not set dates yet. There are nightly wheels up if you want to try now. |
Just a side point, I would not be opposed to adding supports for time and time delta, especially considering that datetime, and timezone already have supports in place for it to work. time is a class that represents a time of day, with no date or time zone information. It has attributes for hour, minute, second, microsecond, and time zone offset timedelta is a class that represents a duration or difference between two dates or times. It can be used to perform arithmetic operations on datetime objects, such as adding or subtracting a certain number of days, hours, minutes, or seconds. |
I'm going to primarily address the type checking side of this problem. The docstring side is an "easier" problem, because we can say whatever we need to there. Not trivial because we don't want to raise the barrier to entry for the simple case, but we have mechanisms to play with. When doing #24976, it was an intentional choice to leave out some of the unit specific behaviors/modifications to signatures (note, for instance, that units.py, category.py and dates.py have no As @tacaswell and @jklymak have pointed out, the units system is actually dynamic behavior as to what is allowed, and therefore essentially reverts to We use In fact, ArrayLike falls short in two main places:
So, my proposal for keeping utility in type hinting while expressing intent through type hints: Part 1: Provide types to use and work with
These two types, with consistent usage and docstring equivalents, should solve any problems for users of matplotlib who do not register new types to converters. Users who do so may need to still Part 2: But what about my units types?Converters for types which extends the "Supports Array" protocol should already be sorted for at least the Fundamentally, registering new types is dynamic behavior that cannot be statically type checked without external knowledge. There is some precedent in the ecosystem for supplying a mypy plugin. Numpy provides one that does platform specific checks for available dtypes. I've not implemented one, but the process doesn't seem too hard. Not the cleanest solution, as it may require end users to configure their type checking, especially in the IDE case (for libraries, this can generally be included in the type checker configs, but if those are not found for further downstream code then it could lead to errors being found which shouldn't be) Part 3: Make the configuration more automaticThis is admittedly a bigger change that would need further discussion before implementing. |
I would not be opposed to that, I think what we would first need is a full list of valid objects for MPL Scalar, and if we can agree on the uniformity of such an object, i.e. some universal objects that are accepted in a large number of use cases, that would make a lot more sense to specify accepted types in one place. |
If the converters are typed is is possible to explain to the typing system that those are the types it needs to match? I am not sure how powerful the generics are (and I think there are open PEPs in this area e.g. https://peps.python.org/pep-0695/). We know a number of down-stream libraries register their own unit converters so I do not think part 2 is a corner case. As pointed out above, we care as (or more) about the number of dimensions and shape (and if it matches between inputs) as the type which is not currently expressed in the type system. At the end of the day, because we do have dynamic inputs, My read is that we have to pick between giving false-negatives (current state where the type checkers flag on something they should not) or giving false-positive (using I'm pretty 👎🏻 on vendoring chunks of numpy's typing or of using anything private. |
To be clear, the chunk in particular for the vendoring route is not actually any real implementation, it is only their protocol class definition of a "Nested Sequence": https://github.com/numpy/numpy/blob/main/numpy/_typing/_nested_sequence.py The implementation would look something like: MplScalar = float | datetime.datetime | datetime.date | datetime.timedelta | Decimal | str | bytes
# Could include int, bool explicitly, but both are actually included in float...
# left out complex for our purposes.
# To use np private apis:
from numpy._typing import _DualArrayLike
MplArrayLike = _DualArrayLike[np.dtype[Any], MplScalar]
# We _could_ restrict the dtype defined types, perhaps, but likely little to no reason to.
# To vendor Nested Sequence, but not rely on _DualArrayLike
MplArrayLike = ArrayLike | MplScalar | _NestedSequence[MplScalar]
# Might work without vendoring, but might have slightly different semantics than ArrayLike's implementation:
T = TypeVar("T")
NestedSequence: TypeAlias = Sequence[Union["NestedSequence[T]", T]]
MplArrayLike = ArrayLike | MplScalar | NestedSequence[MplScalar] The last option there is actually close to what we do for "HashableList", for Overloads could work, with some caveats for pyplot generation (which maybe I should just bite the bullet and resolve anyway, should be possible, and it comes up often enough already...) Generics are an interesting idea to play with... would require that the units type (well, an upper bound of the units type, which could have a default) be known at axis(/axes) creation time, but could probably be worked to default to I do think that type hints for e.g. |
In the meantime I can work on the docs, what do we want to say specifically? |
@Higgs32584 your enthusiasm is appreciated, but not sure we have a full decision yet (and it may change with how we address the type checking side of things) I actually think generics can work, playing with the idea a bit: I added a generic over xunits yunits to Axes (don't actually use it yet, was just doing so for type checking instantiation) diff --git a/lib/matplotlib/axes/_axes.pyi b/lib/matplotlib/axes/_axes.pyi
index 15dc1cf255..7c05d6e3f5 100644
--- a/lib/matplotlib/axes/_axes.pyi
+++ b/lib/matplotlib/axes/_axes.pyi
@@ -32,12 +32,15 @@ import matplotlib.streamplot as mstream
import datetime
import PIL
from collections.abc import Callable, Sequence
-from typing import Any, Literal, overload
+from typing import Any, Literal, Generic, overload, TypeVar
import numpy as np
from numpy.typing import ArrayLike
from matplotlib.typing import ColorType, MarkerType, LineStyleType
-class Axes(_AxesBase):
+XUnits = TypeVar("XUnits")
+YUnits = TypeVar("YUnits")
+
+class Axes(_AxesBase, Generic[XUnits, YUnits]):
def get_title(self, loc: Literal["left", "center", "right"] = ...) -> str: ...
def set_title(
self, With results: # Decl separate from assignment is needed for plt.subplots because of tuple return
fig: Figure
ax_hinted_subplots: Axes[datetime.datetime, datetime.datetime]
fig, ax_hinted_subplots = plt.subplots()
ax_hinted: Axes[astropy.units.Quantity, astropy.units.Quantity] = plt.subplot()
ax_hinted_no_generic: Axes = plt.subplot()
fig1, ax_inferred_subplots = plt.subplots()
ax_inferred =<
82D9
/span> plt.subplot()
reveal_type(ax_hinted) # Axes[Quantity, Quantity] by pyright, Axes[Any, Any] by mypy (Astropy not typed, so doesn't check)
reveal_type(ax_hinted_subplots) # Axes[datetime, datetime]
reveal_type(ax_hinted_no_generic) # Axes[Unknown, Unknown] by pyright, Axes[Any, Any] by mypy
reveal_type(ax_inferred_subplots) # Any
reveal_type(ax_inferred) # Axes[Unknown, Unknown] by pyright, Any by mypy No errors in any of this (not that that precludes problems further down the line)... My gut feeling is this could work essentially perfectly for the OO API, at least. I think pyplot may be a little stuck still though, as the valid types are based on hidden state. I think we could just expand to "Any" in pyplot though and say "if you want stricter type checking, use the OO api, that we usually recommend for other reasons anyway" (if it appears more than once in the signature, we can keep the typevar to say "hand me the same kind of thing for each of these", perhaps... but if it only appears once, then it may even warn for the type checker...) |
Documentation Link
No response
Problem
Related: microsoft/pylance-release#4238
Most functions of axes.Axes are marked as accepting floats, but
datetime
objects seem to work just fine. Docstrings should reflect this fact.Suggested improvement
No response
The text was updated successfully, but these errors were encountered: