yamlpath
yamlpath copied to clipboard
Improve py 3.10 support
This PR...
- Attempts to fix (fully or partially) #166.
- Replaces
pep257
(now unmaintained) with its explicit successorpydocstyle
(see pep257's Homepage redirect or its docs' introduction).-
pep257
cannot be used with Python 3.10 or later. -
pydocstyle
support starts at Python 3.6, same as yamlpath, so there is no need to maintain dual imports or somesuch.
-
- Makes minimal documentation changes to meet
pydocstyle
's more extensive PEP 257 rules.- A better way to handle all the
main()
s may be to remove them from__all__
, or otherwise make them private, but I wanted to avoid introducing any meaningful code changes (someone may be importing one of thosemain()
s for all I know). - I removed non-compliant documentation that did not add context/information instead of rewriting it, let me know if that's not acceptable.
- A better way to handle all the
- Replaces single use of
distutils
, a call todistutils.utils.strtobool
, with an internal re-implementation based on the legacy documentation, per PEP 632's advice.-
distutils
imports raise a deprecation warning starting with Python 3.10 (breaking yamlpath), and will be removed with Python 3.11. - Used functionality is identical, and it's a tiny piece of code, so thought it'd be okay to replace wholesale. If not, could instead do a
try import/except shim
so Python 3.9 and older keep using distutils.
-
This PR does not guarantee that yamlpath now supports Python 3.10. It fixes my downstream issues consuming the package, allows linting to work on Python 3.10, and did not seem to break any existing tests on my local build (was unable to run eyaml tests on any version/branch due to strange ruby encoding issues).
Hence, this PR does not mark this package as 3.10-compatible or add a Python 3.10 build.
Sorry for the "drive-by" nature of the contribution, but I am interested in the long-term health of this project, and happy to contribute more to it.
I fully appreciate your help with this update!
When Python 3.10 came out a year or so ago, I tried to just enable 3.10 and build. However, it wasn't so simple and I had to revert my initial attempt. I didn't (still really don't) have the time to deal with what looked like a nontrivial refactoring effort just to add 3.10 support, particularly when 3.10 users were "early adopters" at the time. In my own employment realm, I see most users sticking with OS vendor-supplied Pythons, which are locked-in at 3.6 with some 3.8 adoption.
I was also frustrated by pytest
/pylint
. If I could just focus on the refactoring for 3.10, that'd be a manageable scope and I'd have gotten back to it by now. But pytest
/pylint
keep moving the goal posts on me. One day, all tests are passing. The next day when pytest
/pylint
push out an update, suddenly, lots of tests are failing and/or my coverage drops under 100%. The nontrivial weight of also addressing ever changing testing requirements on top of the refactoring required to support 3.10 put me off.
I'm really happy to see someone else take an interest in this!
When I next get some time to dedicate to yamlpath
, I'll look at getting a local environment that can pass the tests (probably a vagrant box or docker). Hopefully then I can hunt down any other issues with 3.10 not yet addressed.
Regarding pytest
, fully agreed. In my own projects I pin development dependencies and bump them regularly--mostly to avoid this sort of situation. I'll grab the latest versions at the date of the last passing GitHub action build (December 9th, 2021) and use those, at least until I have passing tests and can deal with dev-side breaking changes.
If you'd be open to moving to that model, I can pip freeze > requirements.dev.txt
and tweak the build scripts accordingly as part of this PR.
If you'd be open to moving to that model, I can pip freeze > requirements.dev.txt and tweak the build scripts accordingly as part of this PR.
I'm squarely on the fence with this suggestion.
I have tried pinning the test tool versions before. I learned the hard way that some CI tools -- notably TravisCI -- ignore pinned dev tool dependency versions. I ran head-long into, "The build passes all tests 100% on my box; why is the CI build different and broken?!" I lost so many hours trying to sort that out every time pytest
/pylint
changed their tests or added new ones. It's frustrating. That said, this may no longer matter as I'm no longer using TravisCI (since they sold their business to owners who don't like FOSS). I just haven't revisited the idea of pinning the testing tool versions after leaving TravisCI.
On the other hand, I really appreciate rigorous testing. I extensively use yamlpath in live, multi-million dollar, production environments. This is why code coverage is 100%. As a user myself, I really appreciate knowing that there are no code issues in the tools. When pytest
/pylint
are changed and those changes discover new issues in the code, it strengthens my confidence in the tool to resolve newly-discovered issues.
So, I could go either way on this one. If we do pin the testing tool versions, and it works (unlike with TravisCI), we'll need to institute a cadence when those tools are deliberated updated and any new issues are resolved. Newly discovered bug fixing would need to be the exclusive effort for that iteration, maybe bundled with also updating to the latest version of ruamel.yaml (depending on how much additional effort that change also brings).
Entirely unrelated to the ongoing conversation: I don't allow direct changes to the master branch, which constitute releases. Releases are a process that includes documentation updates, package building/publication, and so on. Would you please change the target to the development branch or a new feature/add-py310-support branch?
In addressing a couple of emergency patch requests, I've inadvertently caused some merge conflicts for this PR. Worse, I repeated your work on PEP 632 without realizing you'd already done it (entirely my fault for not reading your notes in detail). I'll try to clean this up.
No worries, I haven't had time to come back to this yet (wanted to research best practices re: the issues you brought up). Glad the PEP 632 work got done either way 😄
I was unable to push code to your fork, so I pushed directly to this repo, instead. Consequently, I'm opening a new PR that is in effect this PR but from your (updated) fork/branch (now in this repo) into the development branch of this repo. The net effect is the same as completing this PR. If you'd really rather complete this PR rather than cancel it in lieu of the local replacement, please grant me write access to your fork so I can push the merge conflict fix.
Incidentally, I ran all tests in 3.10 and everything passes. In my book, that means your (+ my) work does add Python 3.10 compatibility. I'll re-run all of the tests on Pythons 3.6 through 3.10 to be sure. If the entire barrage passes, we can add a 3.10 badge and related PyPI classifier.
Thanks!
Umm...
It would seem that my effort to push your code into a new branch of this repo in order to setup a new PR somehow resulted in this PR being closed. That was not my intent. In retrospect, I see how that happened... as far as GitHub is concerned, the intended code has indeed ended up in the target branch as defined by this PR. Now I know to use a different branch name when I mean to open a parallel PR.
Well, the code still needs to end up in the development branch before I can setup a release. I'll go open that PR, now.
Just closing the conversational loop here: all tests pass on Pythons 3.6 through 3.10 in the new PR and on my test/build box. As such, I'm going to merge #189 into development and add the requisite 3.10 call-outs. Thank you for contributing! You'll get credit for this in the change log.