Modernize #2
Merged
Modernize #2
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Can be useful to re-run after a while, in case deps have changed
ubuntu-latest was switched over to ubuntu-22.04, because 3.6 is long EOLd actions/setup-python#544 opted not to build that (actions/setup-python#544 (comment)), therefore extract a sub-matrix to run 3.6 on 20.04. For now, 2.7 is still available on ubuntu-latest (actions/setup-python#543) so that should be fine yet.
See yaml/pyyaml#688 (and possibly cython/cython#1720), doesn't look like there's really a better way to fix it than ignore the issue, but that's simple enough, as it's just an additional `-W` flag on the pytest invocation. While at it, add pypy-3.9 and convert versions array to a yaml list for better readability and diff-ability.
Also update for 3.11 final, add 3.12a, and pypy 3.9, and add support for manual trigger.
- src/ layout seems to be the modern standard, and avoids e.g. false-positive success issues (where tests get run agains the source instead of the packaged library, and thus hide packaging issues): https://hynek.me/articles/testing-packaging/ - remove use of unittest entirely (switch everything to pytest, not just runner), also move doctesting to pytest - moving tests out avoids packaging them, and mucking up the source dir with more test files in the future - replace old test command by an invocation of tox The only thing that's lost is `setup.py check`, but turns out: - it only checks that `long_description` is valid and we don't use that - it wasn't being run on CI - invoking `setup.py` directly is getting deprecated
- 2.7 was EOL'd in 2020 - 3.6 was EOL'd in 2021 - 3.7 will be EOL'd in June 2023 Even if a 1.0 is cut before June it doesn't seem likely that it'll matter a lot. 3.8 doesn't bring a lot that's likely to be useful as far as I know, candidates I can think of are: - positional-only parameters - typed dicts and literals, to add typing to the legacy API While at it, remove the now unnecessary future imports, as well as the module-level docstrings (they're not very helpful) and the per-file license headers (they don't seem useful).
PEP 517 ======= Don't migrate away from setuptools, but move "static" packaging content from `setup.py` to `pyproject.toml` and `setup.cfg`. Building should now be performed using [`build`](https://pypa-build.readthedocs.io/en/stable/), as direct invocations of `setup.py` have been deprecated for a while. Also remove the legacy `__author__` strictures from the source files, as `pyproject.toml` supports an authors section which is more suitable. Codegen Modernization ===================== The modernization via command overrides and self-invocation still generated `setup.py` warnings. Migrate the "invoke commands in commands" method to codegen sub-commands in `build`, it looks like this suffices for what we need. This reuses the existing code, though modernizes it significantly thanks to the previous commit. This is strongly inspired by @abravalheri's comments on codegen in setuptools discussions e.g. pypa/setuptools#3180 and pypa/setuptools#3762, those comments were very helpful in getting a better understanding of the sub_commands system (also @jaraco's comments but those were a touch terse so can't say I really got it until sumbling on @abravalheri's). Leverages the setuptools subcommands protocol to support editable wheels, as that might be useful. Don't use editable wheels for testing though, as I don't see the point. Note: not sure the git submodule stuff is useful during the codegen, so currently commented it out. sdists ====== I'm not entirely clear what sdists should really be about, adding codegen to sdist using subcommands seems not necessarily trivial, so for now make them basically an export: - remove `_regexes.py` from sdist - add uap-core to the manifest, and thus the sdist, at least `regexes.yaml` Currently the test files (both python and yaml) are also included, but it's not clear that they *should* be. That seems like [a whole debate](https://discuss.python.org/t/should-sdists-include-docs-and-tests/14578/117), One strong argument (to me) in favor of a sdist maximalism is that it matches the wheels and more official releases, but in that case the sdist should probably be better crafted than what I threw together. Tox === Switch tox to an explicit `pip install` as its builtin handling for installing packages (whether via sdist or develop) does not seem compatible with PEP-517. Need to think about moving to tox 4, which apparently has native support for PEP 517 packages. Cf #157. Also remove the `docs` tox env, it hasn't been a thing since 8273477 but I forgot to remove it from the envlist.
- remove old compile step - update python-action to v4 and "primary" python to 3.11 - produce sdist and wheel just once - ensure we're testing after installing from wheel, sdist, and source
- remove P2 compatibility (e.g. str/bytes) - remove never used `MatchSpans` methods (not sure what they were intended for) - remove usage of `jsParseBits`, remove from caches and inner parsers, deprecate at the outer parser directly - add a few types - modernise code a bit (e.g. match group indexing, f-strings)
Tox 4 is (finally) compatible with wheel installs & stuff, which is especially nice as the manual wheel installation was not really compatible with `tox -p` (jobs could conflict with one another and corrupt an other job's wheel). The issue doesn't seem to happen with `tox p` which is nice, and while it's technically not *shorter* than the old tox conf, it's definitely clearer, and feels more resilient (we'll see if it is in the long run). Also remove `requirements_dev`: - it contained tox, which is kind-of a dev requirement but kind-of not - as a result it caused the installation of tox in the tox envs, and in the github test images, both completely unnecessary - the dev dependencies are pytest and pyyaml, which is shorter than spelling out `-rrequirements.dev` in full
Since the switch to src layout and pytest in 8273477 the main tests had not been running at all, as they don't match the pytest naming conventions (thankfully I'd not broken anything). - rename the two problematic test classes to be picked up by pytest - also handle the warning generated by `GetFilters` since a24779b - and remove the warnings configuration in `TestDeprecationWarnings` as it has not been necessary since a24779b (P2 being dropped), and possibly even 8273477 (as I wouldn't be surprised if pytest did the right thing on P2 either)
This is broken, but obviously not tested.
The label can be selected with `-m` on most tox commands, this is equivalent to selecting the corresponding envs using `-e`. - `test` runs all the tests, in all python versions - `check` runs the non-test checks - `pypy` and `cpy` run the tests for their respective Python implementation This is way more convenient when leveraging `posargs` as most of the tools are not posargs-compatible. Also easier than typing the envs in full. The only drawback is `tox list` does not display the labels. Also use brace expansions for cleaner definitions (and easier updates), in both envlist and labels.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.