ENH: implement editable loader with PEP 829 .start files - #881
Conversation
e372b81 to
48a9d49
Compare
rgommers
left a comment
rgommers
left a comment
There was a problem hiding this comment.
Thanks @dnicolodi. This LGTM, and it seems to work as advertised. One tiny stylistic comment.
I considered the "ship both" vs. "gate on >=3.15" - both are defensible, so happy to go with your preference here.
I also considered whether this needs more test coverage. A small test that actually checks whether .start is present and .pth is not could be added, however the current editable tests cover perfectly well that things are working and the chance that we'll regress to shipping a .pth file seems quite low, so it's fine as is.
Should be good to merge as is, or after addressing the small style comment.
48a9d49 to
5c0abd1
Compare
I hesitated adding this test when I implemented the PR. Given that you had the same instinct, I added it.
I went for a small refactoring that removed the indirection through the |
rgommers
left a comment
rgommers
left a comment
There was a problem hiding this comment.
Refactoring LGTM. One other very minor comment around readability. The commit message of the second commit is incomplete, so it needs squashing.
| def test_editable_contents(editable_simple): | ||
| artifact = wheel.wheelfile.WheelFile(editable_simple) | ||
|
|
||
| impl = 'start' if sys.version_info > (3, 15) else 'pth' |
There was a problem hiding this comment.
> (3, 15) is actually correct, but I had to do a double take here; >= (3, 15) behaves identically and matches the implementation and intuition better, so I'd change it to that.
There was a problem hiding this comment.
It is practically correct but semantically incorrect.
Given that there is never actually a 3.15 release, only a 3.15.0 release, comparing this as a tuple of ints instead of as an "abstract version number" will always see the side with trailing bits as greater. However technically it is wrong to assume that you're checking for "at least patchlevel greater than <omitted argument>" (even NULL is sorta ironically not a good fit to model this case), quite aside from the intuition aspect.
on Python 3.15 and later. Editable wheels are Python version specific, thus there is no need to write both a .pth and a .start file for Python versions that support the latter. Editable wheels implemented with .pth files just imported the implementation module and relied on the execution of the module body on import for the installation of the module loader. PEP 829 .start files need to specify a callable that takes no arguments. To keep the difference between .pth and .start file implementations to a minimum, define such callable in the implementation module and switch the .pth file to explicitly call it. Fixes mesonbuild#846
5c0abd1 to
f50e3a5
Compare
That was actually a typo.
And I fat fingered something to get the silly commit message, the intention was fixup the previous commit. Thanks for the review and for merging. |
3 participants
on Python 3.15 and later. Editable wheels are Python version specific, thus there is no need to write both a .pth and a .start file for Python versions that support the latter.
Editable wheels implemented with .pth files just imported the implementation module and relied on the execution of the module body on import for the installation of the module loader. PEP 829 .start files need to specify a callable that takes no arguments. To keep the difference between .pth and .start file implementations to a minimum, define such callable in the implementation module and switch the .pth file to explicitly call it.
Fixes #846