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

Vendor typeguard 2.13.3 #142

Merged
merged 15 commits into from
Jan 5, 2024
Merged

Conversation

willhardy
Copy link
Contributor

@willhardy willhardy commented Jan 2, 2024

Vendor the old version of typeguard that is used (2.13.3), so that projects are free to upgrade or not install typeguard as needed (eg for other packages that depend on newer versions).

I have tried to make this as smooth as possible, to require project maintainers to make as few changes as possible and to allow it to work in as many environments as possible. To do that, I have edited the pytest plugin to allow projects to continue to use --typeguard-packages if typeguard is not installed, or if a compatible version of typeguard is installed.

If an incompatible version of typeguard is installed, project maintainers will need to use --stp-typeguard-packages.

How to test

pip install -e .
pip uninstall typeguard
pytest --typeguard-packages=tests
pytest --stp-typeguard-packages=tests
pytest --typeguard-packages=tests --stp-typeguard-packages=tests  # expect exception

pip install typeguard==2.13.3
pytest --typeguard-packages=tests
pytest --stp-typeguard-packages=tests

pip install typeguard==4.1.5
pytest --stp-typeguard-packages=tests
pytest --typeguard-packages=tests  # expect fail

Double check the typeguard code is correct:

pip install python-vendorize
rm -fr strictly_typed_pandas/_vendor/*
python-vendorize
git diff  # There were some minor changes made, inspect them

To do...

  • Update documentation
  • Check if a compatible version of typeguard is installed, and to allow --typeguard-packages to be checked by that version.
  • I've tried to exclude the vendored code from pre-commit, but when it runs for "all files" the vendored code is checked. I tried to add configuration for each of the tools, but it wasn't always picked up. Updating the tool configuration is a better idea, because it will also work when the tools (eg mypy) are run manually (ie not via pre-commit).
  • Allowing --typeguard-packages to be used is a convenience, but it will silently fail in the following situation. We should try to identify that situation, and raise an exception:
    • end developer is using strictly_typed_pandas and wants to make use of typeguard
    • has a newer (unsupported) version of typeguard installed (eg 3+)
    • uses --typeguard-packages for those packages where they should be using --stp-typeguard-packages
    • UPDATE: I don't think this is possible, so I'll recommend only the --stp-typeguard-packages option in the documentation and hope for the best. We could alternatively output a python warning that the user can ignore if they know what they're doing.

This package will however need to be edited to work in parallel with other installations of typeguard.
If typeguard is not installed separately, process the packages listed under --typeguard-packages using the vendored typeguard. Otherwise, process only what is listed in --stp-typeguard-packages and leave the --typeguard-packages option to be processed by the installed version of typeguard. To avoid the ambiguity of double processing, an exception is raised if a package is listed in both options.
@willhardy willhardy mentioned this pull request Jan 2, 2024
@willhardy willhardy marked this pull request as ready for review January 2, 2024 17:21
Copy link
Owner

@nanne-aben nanne-aben left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @willhardy, this looks great!

I think we should add a page to the documentation specifically on typeguard, which would explain:

  1. Why we vendored typeguard
  2. How you can run stp with typeguard 2 installed (backward compatibility)
  3. How you can run stp with the vendored version
  4. How you can use newer versions of typeguard in parallel to stp

Would be happy to contribute to this, let me know what you prefer!

On a related note, does the import hook work? I guess you cannot use it in both the vendored version and typeguard >=3 at the same time, right? So maybe we should just discuss it in the documentation in the backward compatibility section, and specify that it is not exposed in the vendored version?

.pre-commit-config.yaml Show resolved Hide resolved
@nanne-aben nanne-aben merged commit 3924841 into nanne-aben:main Jan 5, 2024
6 checks passed
@nanne-aben
Copy link
Owner

I've merged the PR, but the documentation didn't build as I expected. Will fix that tonight and then I'll release a new version of stp.

@nanne-aben
Copy link
Owner

I've released the new version. Thanks for your contribution @willhardy !

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.

2 participants