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

settings: remove old mypy workarounds #1029

Merged
merged 5 commits into from
Nov 24, 2023

Conversation

woodruffw
Copy link
Member

These workarounds are no longer necessary on recent versions of mypy, and are causing CI failures (due to redundant casts).

Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
Copy link
Member Author

Ah, this is a bit of a pain: the workaround is no longer needed on newer versions of mypy with newer Python versions. Looking into this more.

@woodruffw
Copy link
Member Author

woodruffw commented Nov 23, 2023

Looks like this is the snarl: the version of mypy needed (1.7.0) supports only Python 3.8 and higher, but twine still supports Python 3.7.

Some options here:

  • Bump the minimum Python to 3.8 (3.7 is EOL)
  • Limit typechecking in CI to only 3.8 and higher
  • Double down on the workarounds here and tell mypy to ignore its own redundant cast warnings.
  • Retain the workaround and pin mypy < 1.7.0

@sigmavirus24
Copy link
Member

We aren't deliberately trying to keep 3.7 alive, just like we aren't deliberately not testing on 3.11 or 3.12,so I think we can drop 3.7

@woodruffw
Copy link
Member Author

We aren't deliberately trying to keep 3.7 alive, just like we aren't deliberately not testing on 3.11 or 3.12,so I think we can drop 3.7

Sounds good. I can drop 3.7 and add 3.{11,12} in this PR.

Also adds 3.10 through 3.12, where missing.

Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
Signed-off-by: William Woodruff <william@yossarian.net>
@woodruffw
Copy link
Member Author

Docs error looks like a re-manifestation of #838:

WARNING: error while formatting arguments for twine.package.Hexdigest.__new__: Handler <function update_defvalue at 0x7f1e066c7b80> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for twine.utils.get_cacert: Handler <function update_defvalue at 0x7f1e066c7b80> for event 'autodoc-before-process-signature' threw an exception (exception: )
WARNING: error while formatting arguments for twine.utils.get_clientcert: Handler <function update_defvalue at 0x7f1e066c7b80> for event 'autodoc-before-process-signature' threw an exception (exception: )
looking for now-outdated files... none found
pickling environment... done
checking consistency... done
preparing documents... done
copying assets... copying static files... done
copying extra files... done

Signed-off-by: William Woodruff <william@yossarian.net>
Comment on lines +4 to +6
# Remove this upper bound when twine's minimum Python is 3.9+.
# See: https://github.com/sphinx-doc/sphinx/issues/11767
Sphinx>=6,<7.1
Copy link
Member Author

Choose a reason for hiding this comment

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

NB: This is my temporary hack for working around sphinx-doc/sphinx#11767.

Another option here would be to do the docs build in CI on Python 3.9 or newer, since those don't seem to be affected. But I wasn't sure if tox -e docs was expected to work on all supported Python versions or not 🙂

@woodruffw
Copy link
Member Author

The last lingering failure here is in the integration suite; TestPyPI is returning 503s. That doesn't look localized to this PR, so I think that will just need to wait (or be ignored) 🙂

@sigmavirus24
Copy link
Member

Odd I could have sworn we were using devpi for a server local to that test

@woodruffw
Copy link
Member Author

Odd I could have sworn we were using devpi for a server local to that test

Looks like there's test_devpi_upload and also test_pypi_upload, and only the former uses a local devpi instance.

@woodruffw
Copy link
Member Author

Another oddity: TestPyPI returned 503, but the dist is present: https://test.pypi.org/project/twine-sampleproject/3.0.0.post20231123223830060228/

@sigmavirus24
Copy link
Member

Very odd. I think I'm going to merge this despite that

@sigmavirus24
Copy link
Member

Ah, guess I can't force merge this.

@woodruffw
Copy link
Member Author

Post-hoc triage: the 503 in the integration tests here was caused by pypi/warehouse#14928. I'll fix that.

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