Skip to content

MINOR: [Python] Mention interval in the is_temporal docstring - #51016

Open
Nishuuzz wants to merge 1 commit into
apache:mainfrom
Nishuuzz:minor-is-temporal-doc
Open

MINOR: [Python] Mention interval in the is_temporal docstring#51016
Nishuuzz wants to merge 1 commit into
apache:mainfrom
Nishuuzz:minor-is-temporal-doc

Conversation

@Nishuuzz

Copy link
Copy Markdown
Contributor

pyarrow.types.is_temporal() returns True for interval types, but its docstring doesn't say so:

>>> import pyarrow as pa, pyarrow.types as t
>>> t.is_temporal(pa.month_day_nano_interval())
True
>>> t.is_temporal.__doc__.strip().splitlines()[0]
'Return True if value is an instance of type: date, time, timestamp or duration.'

That's the documented contract being narrower than the behaviour, and since these docstrings are generated from the @doc(is_null, datatype=...) decorator they're what ends up on the API docs page.

Including interval is deliberate rather than accidental — _TEMPORAL_TYPES is built as

_TEMPORAL_TYPES = ({lib.Type_TIMESTAMP,
                    lib.Type_DURATION} | _TIME_TYPES | _DATE_TYPES |
                   _INTERVAL_TYPES)

and test_is_temporal_date_time_timestamp already asserts types.is_temporal(pa.month_day_nano_interval()). So this is a docs fix, not a behaviour question; I've only changed the datatype string.

After the change the rendered line is

Return True if value is an instance of type: date, time, timestamp, duration or interval.

which is exactly the set that returns True — I checked each one (date32, time32, timestamp, duration, interval all True; int32 False) by applying the same edit to an installed pyarrow and reading the generated docstring back.

I found this while comparing every predicate set in types.py against the corresponding arrow::is_*() in cpp/src/arrow/type_traits.h, the same way GH-50847 turned up. Worth noting the two is_temporal definitions genuinely differ — the C++ one covers only date/time/timestamp, while Python also counts duration and interval — but that's a semantic question rather than something to change quietly in a MINOR PR, so I've left it alone and only made the Python docstring match the Python behaviour.

`pyarrow.types.is_temporal()` returns True for interval types, since
`_TEMPORAL_TYPES` is built from `_INTERVAL_TYPES` along with the date,
time, timestamp and duration ids, and `test_is_temporal_date_time_timestamp`
asserts that it does. The generated docstring only listed "date, time,
timestamp or duration", so the documented contract was narrower than the
behaviour.

Add interval to the list. No behaviour change.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant