Test against pydantic 2.0 prereleases

[jwodder]

Part of #176.

Note that we cannot make the code compatible with both pydantic 1.x and 2.x, as all pydantic instance methods have been renamed.

[codecov[bot]]

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.01 :tada:

Comparison is base (845cb17) 97.71% compared to head (37965fe) 97.72%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## master #178 +/- ## ========================================== + Coverage 97.71% 97.72% +0.01% ========================================== Files 17 17 Lines 1751 1761 +10 ========================================== + Hits 1711 1721 +10 Misses 40 40 ``` | Flag | Coverage Δ | | |---|---|---| | unittests | `97.72% <100.00%> (+0.01%)` | :arrow_up: | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#carryforward-flags-in-the-pull-request-comment) to find out more. | [Impacted Files](https://app.codecov.io/gh/dandi/dandi-schema/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi) | Coverage Δ | | |---|---|---| | [dandischema/digests/zarr.py](https://app.codecov.io/gh/dandi/dandi-schema/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#diff-ZGFuZGlzY2hlbWEvZGlnZXN0cy96YXJyLnB5) | `98.78% <100.00%> (+0.04%)` | :arrow_up: | | [dandischema/metadata.py](https://app.codecov.io/gh/dandi/dandi-schema/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#diff-ZGFuZGlzY2hlbWEvbWV0YWRhdGEucHk=) | `96.66% <100.00%> (+0.03%)` | :arrow_up: | | [dandischema/models.py](https://app.codecov.io/gh/dandi/dandi-schema/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#diff-ZGFuZGlzY2hlbWEvbW9kZWxzLnB5) | `97.70% <100.00%> (+0.01%)` | :arrow_up: | | [dandischema/tests/test\_models.py](https://app.codecov.io/gh/dandi/dandi-schema/pull/178?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=dandi#diff-ZGFuZGlzY2hlbWEvdGVzdHMvdGVzdF9tb2RlbHMucHk=) | `96.53% <100.00%> (+0.03%)` | :arrow_up: |

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[yarikoptic]

which methods do we actually use and can't we have some "shim" compatibility layer until we simply declare pydantic 2.0 to be the minimal supported version?

I started with this patch but immediately got into incompatible API and stopped -- but could you please check it deeper a bit?

```patch diff --git a/dandischema/compat.py b/dandischema/compat.py new file mode 100644 index 0000000..6ae0b06 --- /dev/null +++ b/dandischema/compat.py @@ -0,0 +1,6 @@ +import pydantic + +class BaseModel(pydantic.BaseModel): + def json(self, *args, **kwargs): + # TODO: TypeError: BaseModel.model_dump_json() got an unexpected keyword argument 'separators' + return super().model_dump_json(*args, **kwargs) \ No newline at end of file diff --git a/dandischema/digests/zarr.py b/dandischema/digests/zarr.py index b7f8eab..7f31a84 100644 --- a/dandischema/digests/zarr.py +++ b/dandischema/digests/zarr.py @@ -7,6 +7,8 @@ from typing import Dict, List, Optional, Tuple import pydantic +from ..compat import BaseModel + """Passed to the json() method of pydantic models for serialization.""" ENCODING_KWARGS = {"separators": (",", ":")} ZARR_CHECKSUM_PATTERN = "([0-9a-f]{32})-([0-9]+)--([0-9]+)" @@ -26,7 +28,7 @@ def parse_directory_digest(digest: str) -> tuple[str, int, int]: @total_ordering -class ZarrChecksum(pydantic.BaseModel): +class ZarrChecksum(BaseModel): """ A checksum for a single file/directory in a zarr file. @@ -42,7 +44,7 @@ class ZarrChecksum(pydantic.BaseModel): return self.name < other.name -class ZarrChecksums(pydantic.BaseModel): +class ZarrChecksums(BaseModel): """ A set of file and directory checksums. diff --git a/dandischema/models.py b/dandischema/models.py index 0d73fca..292d100 100644 --- a/dandischema/models.py +++ b/dandischema/models.py @@ -10,7 +10,6 @@ from typing import Any, Dict, List, Optional, Sequence, Type, TypeVar, Union, ca from pydantic import ( UUID4, AnyHttpUrl, - BaseModel, ByteSize, EmailStr, Field, @@ -20,6 +19,7 @@ from pydantic import ( ) from .consts import DANDI_SCHEMA_VERSION +from .compat import BaseModel from .digests.dandietag import DandiETag from .digests.zarr import ZARR_CHECKSUM_PATTERN, parse_directory_digest from .utils import name2title diff --git a/setup.cfg b/setup.cfg index b71143b..cba00a1 100644 --- a/setup.cfg +++ b/setup.cfg @@ -30,7 +30,7 @@ project_urls = python_requires = >=3.7 install_requires = jsonschema[format] - pydantic[email] >= 1.8.1, < 2.0 + pydantic[email] >= 1.8.1 typing_extensions; python_version < "3.8" requests zip_safe = False ```

[jwodder]

@yarikoptic I'm not certain that trying to support pydantic 1.x and 2.x at the same time is worthwhile.

For the record, the repositories under the dandi org that use pydantic are:

• dandi/dandi-api-datasets: tools/populate_dandiset_asset_yamls.py: Does not specify a version
• dandi/dandi-archive: Does not specify a version. However, the only recursive dependencies that require pydantic are dandischema and zarr-checksum, and so deployment of dandi-archive (assuming the only Python packages involved are those listed in its setup.py and their recursive dependencies) should continue to use pydantic 1.x even after 2.x comes out, up until dandischema and zarr-checksum both permit use with pydantic 2.x.
• dandi/dandi-cli: Requires pydantic >= 1.9.0. However, it depends on dandischema and zarr_checksum, which both specify an upper bound on pydantic of < 2.0 (and these are the only recursive dependencies that require pydantic), and so pip will either complain or (worse) backtrack if a user tries to install both dandi-cli and pydantic 2.0 in the same environment.
  • Honestly, if a user tries to install dandi in an environment that already has pydantic 2.x installed, I'd say they're doing something wrong. We should be strongly encouraging users to only install dandi in a fresh virtualenv or Conda environment; installing a Python package at the system or user level is just playing with fire.
• dandi/dandi-schema: Now requires pydantic >= 1.8.1, < 2.0
• dandi/dandisets:
  • tools/backups2datalad/: Requires pydantic >= 1.8, < 2.0
  • tools/get-upload-status.py: Requires pydantic >= 1.7, < 2.0
• dandi/dandisets-healthstatus: Requires pydantic >= 1.8, < 2.0
• dandi/nwb-healthstatus: Requires pydantic >= 1.7, < 2.0
• dandi/zarr_checksum: Requires pydantic >= 1.10.2, < 2.0

[yarikoptic]

• We should be strongly encouraging users to only install dandi in a fresh virtualenv or Conda environment

overall: we can't really encourage such things, especially because dandi is also a library. E.g. if some tool (or even downstream internal script) would requires already pydantic 2.0 or would not be ready to upgrade to it yet -- we can't just demand them to either use outdated dandi (which might not be "good enough") or downgrade back to 1.x pydantic since dandi is not ready yet. IMHO it was quite a bad decision from pydantic team to just break API without some compatibility shim with a DeprecationWarning and thus a more "gentle" deprecation cycle.

[yarikoptic]

FWIW: I filed https://github.com/pydantic/pydantic/issues/5792

[yarikoptic]

@jwodder - we got an answer at https://github.com/pydantic/pydantic/issues/5792#issuecomment-1562687862 and presumably with https://github.com/pydantic/pydantic/pull/5480 merged , I believe as http://github.com/samuelcolvin/pydantic/commit/e78dd92cf896797215d6d9870511a2d89941f87b which is included in the v2.0a3~4. Could you please look at feasibility to keep dandischema compatible with both v1 and v2 by keeping using pydantic.v1 from that v2.0 pre-release?

[jwodder]

@yarikoptic I believe that using any v1-specific features under v2 will result in DeprecationWarnings, which will have to be ignored by the pytest configurations for dandischema, dandi-cli, and any other similarly-configured test suites.

[yarikoptic]

Ignoring by test suites is IMHO an acceptable cost during transition

[jwodder]

@yarikoptic Remind me why we want to support both v1 and v2 at the same time in the first place.

[yarikoptic]

so we could avoid needing < 2.0 in version specification of all components and bring them all up to compatibility with v2 in due time without needing to orchestrate transition of them at the same time. I see it as

• do introduce <2.0 where they aren't compatible with v2 ( so we better do that ASAP anyways)
• for compat with 1 and 2, introduce use of shims if version >2.0a3 and remove <2.0 one component at a time, right away
• whenever all components have releases with <2.0 removed -- stop using the shim in them one at a time.

IMHO this would be smoother than striving to do migration in 1 go across all of those and who knows what to do with downstreams.

[jwodder]

@yarikoptic At the moment, the pydantic.v1 namespace (added to the codebase on May 17) is not present in the latest pydantic 2.0 alpha release (made on May 5), so I cannot test support for both versions.

[yarikoptic]

so I cannot test support for both versions.

can always be installed directly from git for testing, e.g. even from some specific committish to avoid "fluctuations" and later switch to pre-release whenever a2 is out.

[yarikoptic]

FWIW, I thought to ask for a2 but it seems that overall pydantic release seems to be close, at least according to https://github.com/pydantic/pydantic/issues/5919 , so we better hurry with adding all the version restrictions and v1/v2 compatibility shimming.

[jwodder]

@yarikoptic I've gotten the tests to pass with both v1 and v2 of pydantic, but the type-checking is failing with both. I've posted https://github.com/pydantic/pydantic/discussions/5971 to ask for a solution.

[yarikoptic]

@jwodder , I don't want the typing check oddity to stop us prepare in time. Could we just skip that check inline in that location for now and get back to it whenever mypy folks reply?

[jwodder]

@yarikoptic There are multiple failing checks all over the code, and it's only going to get worse when we try to apply this approach to other dandi packages.

[satra]

closing this in relation to #203