Skip to content
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

Bump jsonschema to 4.19.* #10583

Merged
merged 6 commits into from
Sep 23, 2023
Merged

Bump jsonschema to 4.19.* #10583

merged 6 commits into from
Sep 23, 2023

Conversation

srittau
Copy link
Collaborator

@srittau srittau commented Aug 15, 2023

Closes: #10417

@github-actions

This comment has been minimized.

@srittau
Copy link
Collaborator Author

srittau commented Aug 15, 2023

Depends on typeshed-internal/stub_uploader#100.

@srittau
Copy link
Collaborator Author

srittau commented Aug 15, 2023

I think this is the first package where a dependency doesn't work with all Python versions we support. I guess we need a new METADATA.toml field minimum_python_version or similar.

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 15, 2023

I think this is the first package where a dependency doesn't work with all Python versions we support. I guess we need a new METADATA.toml field minimum_python_version or similar.

New versions of numpy don't support Python 3.7, which is why we run mypy_test.py on Python 3.7 these days, rather than just with --python-version 3.7. In theory, pip should notice that we're using Python 3.7, and refuse to install a version of the package that declares it needs Python 3.8 or higher. This works fine when testing types-tensorflow, which has a dependency on numpy; mypy_test.py just installs a very old version of numpy into the dynamically created venv when testing types-tensorflow, a version of numpy that still supports Python 3.7.

So I'm curious to see exactly what the issue is here...

@srittau
Copy link
Collaborator Author

srittau commented Aug 15, 2023

I assume (not investigated yet) that referencing doesn't declare a dependency on python >= 3.8.

@AlexWaygood
Copy link
Member

I'm looking at it

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 15, 2023

Maybe we should print this to the terminal when the test fails even if the --verbose flag isn't specified for mypy_test.py. It's pretty hard to figure out why things are failing in cases like this without it:

typeshed/tests/mypy_test.py

Lines 299 to 302 in 54193d5

if non_types_dependencies and args.verbose:
print("Ran with the following environment:")
subprocess.run([venv_info.pip_exe, "freeze", "--all"])
print()

@AlexWaygood
Copy link
Member

Okay, so it's installing referencing==0.8.9 in CI on Python 3.7 (not the latest version of referencing -- the latest version is referencing==0.30.2, and the latest version does indeed declare that it only supports Python 3.8+).

@AlexWaygood
Copy link
Member

AlexWaygood commented Aug 15, 2023

And, yup, referencing==0.8.9 is incompatible with Python 3.7, despite the fact that it has no requires-python = ">=3.8" marker in its pyproject.toml file yet (that was only added in later versions). The issue is this function here: https://github.com/python-jsonschema/referencing/blob/a37d9049ce9c78291b7e716f764a7d9790667ed6/referencing/_core.py#L181-L189

    def resource_at(
        self,
        uri: str,
    ) -> tuple[IdentifiedResource, PMap[str, AnchorType], Registry]:
        at_uri = self._contents.get(uri)
        if at_uri is None or uri in self._uncrawled:
            registry = self._crawl()
            return *registry._contents[uri], registry
        return *at_uri, self

Those return statements parse as valid syntax on Python 3.8, but not 3.7.

@github-actions

This comment has been minimized.

@AlexWaygood
Copy link
Member

I guess we need a new METADATA.toml field minimum_python_version or similar.

We should be able to merge #10559 as soon as the PyPI admins let mypy publish a 1.5 patch release fixing the stubtest regression. (Or we could just allowlist the stubtest regression for now, and unallowlist it as soon as mypy 1.5.1 comes out.) Once #10559 is merged, we stop running mypy_test.py on Python 3.7, so the problem sort-of becomes moot.

We're only in this situation in the first place because the referencing didn't dot all the I's in its packaging metadata for its early releases, so it feels unlikely it'll come up too much.

@AlexWaygood
Copy link
Member

We unfortunately can't pin to any version of referencing earlier than v0.8.9 (even if they're compatible with Python 3.7, which I haven't checked), since v0.8.9 is the first version of referencing that has a py.typed marker.

@srittau srittau added the status: deferred Issue or PR deferred until some precondition is fixed label Aug 15, 2023
@srittau
Copy link
Collaborator Author

srittau commented Aug 15, 2023

Thanks for the analysis! Let's wait for mypy 1.5 then and see if that helps.

@github-actions

This comment has been minimized.

@srittau srittau removed the status: deferred Issue or PR deferred until some precondition is fixed label Aug 16, 2023
@github-actions
Copy link
Contributor

According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants