Pydantic >=2.0 support

[xaviernogueira]

Overview

• Pydantic 2.0 requires classes that override a BaseModel's fields to have the overridden value type annotated. I added these small type annotations for the name field, and also added information to the docs.
• Additionally, I took a moment to better understand the code base, and type hinted all functions appropriately as well as applying some minor code readability improvements.
• I also removed the -r requirements.txt syntax in dev-requirements.txt such that one can install via conda, mamba, or pip. All dependencies are in alphabetical order.

[abkfenris]

Overview

* Pydantic 2.0 requires classes that override a `BaseModel`'s fields to have the overridden value type annotated. I added these small type annotations for the `name` field, and also added information to the docs.
* Additionally, I took a moment to better understand the code base, and type hinted all functions appropriately as well as applying some minor code readability improvements.

* I also removed the `-r requirements.txt` syntax in `dev-requirements.txt` such that one can install via conda, mamba, or pip. All dependencies are in alphabetical order.

Does conda or mamba not pick up the -r syntax if you try to use the file directly? For development, I've been making a hot mess of things creating a base conda environment, then using pip install -r dev-requirements and pip install -e ..

To-Dos / Questions before ready for review @abkfenris

* Figure out why there are issues running the test suite related to plugin load-in?

I think it's from the | unions in the 3.9 tests, as that syntax was added in 3.10 which is causing Python to fail accessing it.

* My nodejs installation appears to be faulty on my machine. Stuck thinking it is windows 32 even though I have 64 bit. This prevented me from running the full pre-commit, I had to comment out the `ruff` and `prettier` parts. Could you run this for me? Or will the bot do it?

You can ask pre-commit.ci to run the checks for you and push the changes back. See the bottom of https://results.pre-commit.ci/run/github/235846200/1689788417.B7uBcS6vRRy61WJYv_tI8Q

* How is the version encoded? Can I just merge as is? I am not familiar with the `pkg_resources.get_distribution(__name__).version` syntax.

That doesn't need to change. We're generating the version # from git tags. When you run pip install -e . locally, it probably gives you Successfully installed xpublish-0.3.0.post#. setuptools_scm find the last tag (0.3.0), and then counts the number of commits after it. In Github Actions, it is showing up as xpublish-0.0.post1 as the clone is only from the last commit.

* **Figure out why tests are running into issues!** Pretty sure it's a typehint thing interacting with FastAPIs response model, so I may undo some of those changes.

We may want to add to also add to our test matrix to test against both Pydantic 2.0 and lower to make sure we don't break anything in older versions. Which I can take care of, or you can give it a shot if you want.

[xaviernogueira]

Yes conda/mamba will fail with the "-r" syntax if you do it directly as in conda env create -f dev-requirements.txt -n xpublish_dev.

Thanks for the tip on pre-commit CI. I will use it!

I'll be trying to get the tests to work now

[xaviernogueira]

pre-commit.ci autofix

[xaviernogueira]

pre-commit.ci autofix

[abkfenris]

I made a PR with a tweaked test matrix that'll test both Pydantic 1 & 2, so we make sure we don't inadvertently break anything with the upgrades. If you want you can pull it into your PR #216

[xaviernogueira]

Okay all tests and pre-commit checks are working..yay!

Only issue is pydantics 2.0 upgrade has caused an issue with the Sphinx autodoc_pydantic plugin. This issue is on their side, but prevents the docs from building.

I made a Issue over there. I would just try to fix it, but their test suite seems to be a mess rn and I'd hate to break something without realizing it.

If no one responds in a few days I'll give it a go.

I'm converting to a real PR. Feel free to merge in your expanded test matrix? Or maybe just wait until after this is merged.

[abkfenris]

I'll take a fuller look through in a bit, but we could pin Pydantic to <2 in https://github.com/xpublish-community/xpublish/blob/main/docs/requirements.txt for the docs.

[xaviernogueira]

Good idea, forgot the docs have their own dependencies.

Also don't merge quire yet. I'm going to fix some of the warning related to deprecated pydantic methods we are using. Should prevent headaches in the future.

Edit: Done

[xaviernogueira]

So I pinned the docs to the old pydantic and it still fails. This is a tad odd since the autodoc_pydantics TOML file specifies for Pydantic <2.0 (link here).

I tried Pydantics experimental "bump-pydantic" package, which made one small change (905688c). I also manually addressed the only deprecation warning coming from the xpublish codebase directly (9a502bd).

[abkfenris]

Ah, looking at the RTD logs, it's the order things are getting installed in. Since xpublish is being installed last, pip ends up upgrading Pydantic then.

xpublish/.readthedocs.yml

Lines 21 to 25 in b425bc1

	python:
	install:
	- requirements: docs/requirements.txt
	- method: pip
	path: .

We specify our install here. I think we may be able to tweak the package install step to install the docs requirements afterwards, or add another step after it (build.jobs.post_install).

 python: 
   install: 
     - method: pip 
       path: . 
    - requirements: docs/requirements.txt 

[xaviernogueira]

All is fixed 💯

[abkfenris]

I think 9a502bd broke Pydantic 1.x compatibility. We'll probably have to live with that deprecation until the rest of the ecosystem is Pydantic 2 compatible.

I found that by testing locally with nox, but you could also grab my PR that changes the Github Actions test matrix to see.

# noxfile.py

import nox

python_versions = [
    "3.9", 
    #"3.10",
    #"3.11"
]

@nox.session(python=python_versions)
@nox.parametrize("pydantic", ["<2", ">=2"])
def tests(session: nox.Session, pydantic: str):
    """Run py.test"""
    session.install("-r", "dev-requirements.txt")
    session.install(".")
    session.install(f"pydantic{pydantic}")
    session.run("pytest", "--verbose")

[xaviernogueira]

Sounds good, the old methods (BaseModel.dict()) still works but will be gone in Pydantic 3.0...we definitely have plenty of time.

So no big issue. So I guess options are:

• Live with the deprecation warning.
• Catch/hide the warning.
• Make method dependent on pydantic version? Either via a try/except or some wonky method like below.

TO_DICT_METHODS = {
     '1': 'dict',
     '2': 'dump_model',
}

...
getattr(self, TO_DICT_METHODS[pydantic.__version__[0]])()

I'll leave the call up to you?

[abkfenris]

How about an exception?

try:
    self.dump_model()
except AttributeError
    self.dict()

[abkfenris]

Most of it looks pretty good. There are a lot of changes that I guess are probably due to Black or Ruff updates, but thats fine.

The one thing that I'm unsure about is the changes to all of the response types. In most cases we'll want to specify the Pythonic type so that FastAPI can document things appropriately. Maybe we can set the default response class for the Zarr APIRouter for the way that Zarr expects JSON.

[abkfenris]

I think these all should probably be dicts as response types, unless we want to make Pydantic models to represent Zarr (and maybe encapsulate encoding)?

Or there is an existing Pydantic Zarr package we could build on...but not something for this PR.

[xaviernogueira]

The idea of encapsulating dict response types if definitely interesting and I agree outside the scope of this PR (also apologies for scope-creep...I got carried away!).

In general I am big into TypedDicts as they don't hard enforce but you can know what to expect somewhat and help contributors. That said, the Pydanic Zarr package seems very interesting. It seems you need to use their pydantic_zarr.GroupSpec.from_zarr() which may introduce a performance hit and appears to need to be done for each Zarr specifically. In that way it may not be ideal. But I just took a quick glance.

Maybe encapsulation could be a topic of a meeting at some point in the future? My next priority on xpublish will be the catalog plugin. I'm going to open an issue where we can start a discussion.

[abkfenris]

Great! Thank you!

Let me know when you are ready to land it.

[xaviernogueira]

Great! Thank you!

Let me know when you are ready to land it.

Just waiting on the tests/docs to run for sanity but I'm ready to go!

[abkfenris]

That loss in Codecov is fine for now, as the except is only hit for Pydantic v1.

I've tested it against v1 locally and it works. I can land my PR after this that will test against both v1 and v2 to help us maintain compatibility in the future, and maybe set up Codecov to merge coverage runs.