Skip to content

Trying to get a compliant, typed version parser/comparator #348

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

Closed
wants to merge 1 commit into from

Conversation

tony
Copy link
Member

@tony tony commented Jan 11, 2022

Companion: tmux-python/tmuxp#727

See also:

@neutrinoceros
Copy link

Hi @tony
So in response to your question in the yt issue, I'm actually not sure my solution version_tuple fits your needs, if you want to support version schemes that used to rely on distutils.LooseVersion, my function may not work.

version_tuple is explicitly aimed only at parsing int tuples, and will truncate any trailing str to support alphas/betas

for instance

>>> version_tuple("2.13-hello")
(2, 13)

is this really what you need ?

@tony
Copy link
Member Author

tony commented Jan 15, 2022

@neutrinoceros I saw your follow up here and realize it is explicitly tuple related and I'm dabbling with keeping it as an object: #351. I've underestimated the effort required.

I suppose if it takes no time at all, you could PR the function in src/libtmux/util.py

@neutrinoceros
Copy link

Seems like you got a better solution from the packaging thread, I won't open a PR after all. Anyway feel free to copy my solution in case you need it still.

@tony
Copy link
Member Author

tony commented Jan 16, 2022

Seems like you got a better solution from the packaging thread, I won't open a PR after all. Anyway feel free to copy my solution in case you need it still.

Thank you @neutrinoceros! @layday's mixin for Version seems quite nice and I'm trialing it in another PR here.

In case anyone googles their way here, @layday's snippet:

from functools import total_ordering

from packaging.version import Version


@total_ordering
class _VersionCmpMixin:
    def __eq__(self, other: object) -> bool:
        if isinstance(other, str):
            other = self.__class__(other)
        return super().__eq__(other)

    def __lt__(self, other: object) -> bool:
        if isinstance(other, str):
            other = self.__class__(other)
        return super().__lt__(other)


class MyVersion(_VersionCmpMixin, Version):
    pass


assert MyVersion("3") == "3.0.0"
assert MyVersion("3") != "3.0.1"

@tony tony force-pushed the python-3.10-deprecation branch from ec9aed4 to 42bc294 Compare February 26, 2022 17:35
@codecov
Copy link

codecov bot commented Feb 26, 2022

Codecov Report

Merging #348 (42bc294) into master (5166809) will decrease coverage by 0.76%.
The diff coverage is 88.23%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #348      +/-   ##
==========================================
- Coverage   87.82%   87.05%   -0.77%     
==========================================
  Files          15       15              
  Lines        1511     1530      +19     
==========================================
+ Hits         1327     1332       +5     
- Misses        184      198      +14     
Impacted Files Coverage Δ
libtmux/common.py 85.13% <88.23%> (-1.80%) ⬇️
tests/test_common.py 92.17% <0.00%> (-6.09%) ⬇️
tests/test_server.py 98.18% <0.00%> (-1.82%) ⬇️
tests/test_window.py 98.14% <0.00%> (-0.62%) ⬇️
libtmux/session.py 81.32% <0.00%> (-0.23%) ⬇️
libtmux/window.py 85.36% <0.00%> (+1.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5166809...42bc294. Read the comment docs.

@tony tony force-pushed the master branch 2 times, most recently from bb68b70 to d6ab13b Compare July 14, 2022 10:33
@tony tony closed this in e3e6c39 Dec 10, 2022
@tony tony deleted the python-3.10-deprecation branch December 10, 2022 20:58
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