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

[stubsabot] Bump setuptools to 69.0.* #11049

Closed
wants to merge 1 commit into from

Conversation

github-actions[bot]
Copy link
Contributor

@github-actions github-actions bot commented Nov 21, 2023

Release: https://pypi.org/pypi/setuptools/69.0.2
Homepage: https://github.com/pypa/setuptools
Repository: https://github.com/pypa/setuptools
Changelog: https://setuptools.pypa.io/en/stable/history.html
Diff: pypa/setuptools@v68.2.2...v69.0.2

Stubsabot analysis of the diff between the two releases:

  • 1 public Python file has been added: setuptools/modified.py.
  • 0 files included in typeshed's stubs have been deleted.
  • 26 files included in typeshed's stubs have been modified or renamed.
  • Total lines of Python code added: 627.
  • Total lines of Python code deleted: 457.

If stubtest fails for this PR:

  • Leave this PR open (as a reminder, and to prevent stubsabot from opening another PR)
  • Fix stubtest failures in another PR, then close this PR

Note that you will need to close and re-open the PR in order to trigger CI

@github-actions github-actions bot added the bot: stubsabot 🤖 PRs created by stubsabot 🤖 label Nov 21, 2023
@AlexWaygood AlexWaygood reopened this Nov 21, 2023
@JelleZijlstra
Copy link
Member

I'm not sure this is right; setuptools made a release that made it so they pick up py.typed by default, but I don't see anything in their code that indicates they are shipping a py.typed for their own code.

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 21, 2023

Hah, okay, it looks like the wheel for the latest setuptools release does indeed include a py.typed file, which is causing this function call to return True:

with zipfile.ZipFile(body) as zf:
return any(Path(f).name == "py.typed" for f in zf.namelist())

After a bit of interactive debugging, however, I have identified that there are the following py.typed files in the latest setuptools wheel. Obviously none of these are enough for the whole setuptools package to be understood by type checkers as py.typed :)

pkg_resources/_vendor/importlib_resources/py.typed
pkg_resources/_vendor/more_itertools/py.typed
pkg_resources/_vendor/packaging/py.typed
pkg_resources/_vendor/platformdirs/py.typed
setuptools/_vendor/importlib_metadata/py.typed
setuptools/_vendor/importlib_resources/py.typed
setuptools/_vendor/more_itertools/py.typed
setuptools/_vendor/packaging/py.typed
setuptools/_vendor/tomli/py.typed

We clearly need to make stubsabot a bit smarter here.

@AlexWaygood
Copy link
Member

I suppose the stubsabot logic here was valid for packages built with setuptools<69.0.0. For those packages, you could generally guarantee that py.typed files included in the wheel were relevant, since users had to explicitly opt in to including them in the wheel. But with packages built with setuptools>=69.9.0 (and setuptools==69.9.0 is of course built with setuptools>=69.9.0), that's no longer true.

@JelleZijlstra
Copy link
Member

I feel stubsabot should check that py.typed is present in all of the top-level directories installed by the package. (But maybe that's wrong for namespace packages?)

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 21, 2023

I feel stubsabot should check that py.typed is present in all of the top-level directories installed by the package. (But maybe that's wrong for namespace packages?)

Yes, that would be wrong for (at least some) namespace packages... inspecting the wheel for protobuf, it has this structure:

protobuf/
    google/
        protobuf/
            any_pb2.py
            api_pb2.py
            descriptor.py
            descriptor_database.py
            descriptor_pb2.py
            descriptor_pool.py
            duration_pb2.py
            empty_pb2.py
            field_mask_pb2.py
            json_format.py
            message.py
            message_factory.py
            proto_builder.py
            reflection.py
            service.py
            service_reflection.py
            source_context_pb2.py
            struct_pb2.py
            symbol_database.py
            text_encoding.py
            text_format.py
            timestamp_pb2.py
            type_pb2.py
            unknown_fields.py
            wrappers_pb2.py
            __init__.py
            compiler/
                plugin_pb2.py
                __init__.py
            internal/
                api_implementation.py
                builder.py
                containers.py
                decoder.py
                encoder.py
                enum_type_wrapper.py
                extension_dict.py
                field_mask.py
                message_listener.py
                python_message.py
                testing_refleaks.py
                type_checkers.py
                well_known_types.py
                wire_format.py
                _parameterized.py
                __init__.py
            pyext/
                cpp_message.py
                __init__.py
            testdata/
                __init__.py
            util/
                __init__.py
    protobuf-4.25.1.dist-info/
        LICENSE
        METADATA
        RECORD
        WHEEL

If protobuf were to (miraculously) become py.typed tomorrow, they obviously wouldn't put the py.typed file inside the top-level google/ directory, or they'd be marking all other google PyPI packages such as google-cloud-ndb etc. as py.typed as well. The py.typed file they'd add would go inside google/protobuf/.

@JelleZijlstra
Copy link
Member

However, it's probably better here to have false negatives (stubsabot doesn't recognize a package is obsolete) than false positives (stubsabot incorrectly thinks a package is obsolete). In the former case, we can still manually mark the package as obsolete, but in the latter case, stubsabot becomes unusable for that package. So if we can't come up with a better heuristic, what I proposed is probably still better than the current situation.

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 21, 2023

I'm working on a slightly more sophisticated approach. If you don't like it, then yeah, let's go with what you propose :)

Release: https://pypi.org/pypi/setuptools/69.0.2
Homepage: https://github.com/pypa/setuptools
Repository: https://github.com/pypa/setuptools
Changelog: https://setuptools.pypa.io/en/stable/history.html
Diff: pypa/setuptools@v68.2.2...v69.0.2

Stubsabot analysis of the diff between the two releases:
 - 1 public Python file has been added: `setuptools/modified.py`.
 - 0 files included in typeshed's stubs have been deleted.
 - 26 files included in typeshed's stubs have been modified or renamed.
 - Total lines of Python code added: 627.
 - Total lines of Python code deleted: 457.

If stubtest fails for this PR:
- Leave this PR open (as a reminder, and to prevent stubsabot from opening another PR)
- Fix stubtest failures in another PR, then close this PR

Note that you will need to close and re-open the PR in order to trigger CI
@github-actions github-actions bot changed the title [stubsabot] Mark setuptools as obsolete since 69.0.0 [stubsabot] Bump setuptools to 69.0.* Nov 23, 2023
@AlexWaygood
Copy link
Member

Right, looks like stubsabot is working as expected now, but stubtest is now also confused about there being py.typed files inside subdirectories of pkg_resources._vendor at runtime

@Avasam
Copy link
Collaborator

Avasam commented Nov 23, 2023

Right, looks like stubsabot is working as expected now, but stubtest is now also confused about there being py.typed files inside subdirectories of pkg_resources._vendor at runtime

Which itself is caused by the existence of pkg_resources._vendor only used by that one line at https://github.com/python/typeshed/blob/main/stubs/setuptools/pkg_resources/__init__.pyi#L12

_vendor folders of setuptools are meant to vendor a direct copy of the libraries (note this doesn't include distutils, because it's pypa/distutil, not sdtlib/distutil AND is outside the _vendor folder). Then does some text replacement patches in https://github.com/pypa/setuptools/blob/main/tools/vendored.py . Because of setuptool's bootstraping problem (see: pypa/setuptools#2825). I think we might be able to get away with just importing from packaging directly.
I've made an attempt here: #11066


Edit: Looks like either stubtest or stub_uploader is gonna need an update to keep the packaging subclassing.

@AlexWaygood AlexWaygood deleted the stubsabot/setuptools branch November 28, 2023 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bot: stubsabot 🤖 PRs created by stubsabot 🤖
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants