Accommodate Pandas 3 breaking changes #25
Conversation
| [dependency-groups] | ||
| dev = [ | ||
| "hypothesis>=6.112.1", | ||
| "pytest-httpx>=0.30.0", | ||
| "pytest-asyncio>=0.24.0", | ||
| "pytest-cov>=5.0.0", | ||
| "pytest>=8.3.2", | ||
| ] |
There was a problem hiding this comment.
@snamber What's the reason for removing the dev dependencies here and in the other pyproject toml? is it related to the matrix testing?
There was a problem hiding this comment.
Yes, I pulled all dev dependencies into the root directory so I can install them with uv run --all-groups, I can't install group-scoped dependencies of subdirectory projects
There was a problem hiding this comment.
Have you tried uv run --all-packages too?
| from __future__ import annotations | ||
|
|
There was a problem hiding this comment.
I'd avoid that rabbit hole for now, there was quite a lot of heavy discussion regarding the related pep: https://peps.python.org/pep-0563/ - and in the end it was not accepted, but replaced by another pep.
If you want to read up on it: A history of annotations
But I think it's not needed here, since no other changes are made int his file.
There was a problem hiding this comment.
Without this import there's a typing error, with Pandas 3
There was a problem hiding this comment.
ah yeah, that's a propagation of the other change, can be fixed by quoting: TimeIntervalLike | IDIntervalLike as well, e.g. "TimeIntervalLike | IDIntervalLike"
pyproject.toml
Outdated
| "junitparser>=3.2.0", | ||
| "ty>=0.0.11", | ||
| "prek>=0.2.27", | ||
| # testing |
There was a problem hiding this comment.
Ah now I see, you moved them to the top level, I'd rather avoid doing that if possible, otherwise the individual sub-projects (such as tilebox-datasets are no longer self-contained, but rely on configuration in the repo root).
I'm guessing your issue was that they were not included in the root uv you created by default, which you can achieve by adding the --all-packages flag to the uv sync command, which will include all dependencies (dev and normal) from all packges.
There was a problem hiding this comment.
See the other comment why I consolidated dev dependencies. My thought was that yes, package dependencies should be separate, but this is a monorepo, so dev dependencies can well be consolidated
| # A type alias for the different types that can be used to specify a time interval | ||
| TimeIntervalLike: TypeAlias = ( | ||
| DatetimeScalar | tuple[DatetimeScalar, DatetimeScalar] | xr.DataArray | xr.Dataset | "TimeInterval" | ||
| "DatetimeScalar | tuple[DatetimeScalar, DatetimeScalar] | xr.DataArray | xr.Dataset | TimeInterval" |
There was a problem hiding this comment.
I don't think that's the correct fix here, since class TimeInterval is only defined below, I'll take a look
There was a problem hiding this comment.
Apparently there is one difference between typing.Union and the more modern | operator:
typing.Union[int | float | "SomeString"] works, but int | float | SomeString| doesn't.
Pandas changed from typing.Union to | that's why we got the error now, so the fix for us is to use typing.Union ourselfes.
There was a problem hiding this comment.
Ahh - noticed just now that you quoted the whole thing in a string, that works too, but I feel like that's the less elegant version 😅
There was a problem hiding this comment.
nevermind ty doesn't like the old union with a string in it it seems, so I'll jut go for the full string solution.
No description provided.