Tox dotted env

[gpongelli]

manage dotted env for tox envlist

closes #38

[mgedmin]

Thank you!

[mgedmin]

Can we avoid global state please?

IIIRC I had a function to parse tox envlist names and convert them to Version objects. It should handle optional dots.

The update functions should instead check whether the envlists were using dots or not, just like they currently check whether the envlist uses spaces after commas.

[gpongelli]

the main way I followed when developing was to choose the format to be used in the whole file:

1. if no env has dot, the update should put envs without dot
2. if envs have at least one dot, the update should put envs with dot
3. in mixed case env, it's forced to "change all env with dot"

I know that point 3 is little risky, because developer should update the environments in the remaining part of the file.
I've no idea how to keep "mixed style env" and I don't know what's your point.

[mgedmin]

Sure, I agree with points (1) and (2) and don't care about point (3) -- normalizing to one style is fine, I don't particularly care which style it should be.

My point was that this doesn't need a global switch, a local check inside the update_...() function that then passes the detected style to any helpers should suffice.

[gpongelli]

perhaps I didn't express correctly the drawback about third point : in case of this starting state

[tox]
envlist = py3.8, py3.9, py310, py311
...
[testenv]
setenv =
    {py3.8, py3.9, py310, py311}: <<do something here>>
commands =
    py3.8:  <<do something here>>
    py3.9:  <<do something here>>
    py310:  <<do something here>>
    py311:  <<do something here>>

if the dot isn't stored in some object that identifies what's found in envlist (Version object are created), it can happen that after the update the state becomes (e.g. deciding to force dot removal way)

[tox]
envlist = py38, py39, py310, py311
...
[testenv]
setenv =
    {py3.8, py3.9, py310, py311}: <<do something here>>    <<< HERE NO CHANGES, misalignment with testenv
commands =
    py3.8:  <<do something here>>    <<< HERE NO CHANGES, misalignment with testenv
    py3.9:  <<do something here>>    <<< HERE NO CHANGES, misalignment with testenv
    py310:  <<do something here>>
    py311:  <<do something here>>

as you can see, the envlist is updated removing the dots on 3.8 and 3.9, but the remaining part of the file was not touched , and this led to issue if no review is done AFTER check-python-version was run.

Instead, keeping the 'dot' information as I did, the already existing version into envlist are not changed, so everything still match in testenv section.

I hope that his had clarified the reason about my recent changes.

[mgedmin]

I see. I would hope that tox is smart enough to consider e.g. py38 and py3.8 factors as equivalent, so the configuration would continue to work despite the misalignment.

My view on this remains: preserving a preexisting mixture of styles is extra effort, I don't mind if it's done, but it's not a requirement for the PR to be merged.

Adding mutable global state or a thematically-unrelated attribute to a generic Version class are deal-breakers.

If I wanted to preserve a mixture of styles (which I don't want to do), I'd create a supplementary mapping of Version -> str in update_tox_envlist:

old_spelling = {}
for env in parse_envlist(envlist):
    ver = tox_env_to_py_version(env)
    if ver:
        old_spelling[ver] = env

and then reuse it when computing new_envs:

new_envs = [
    old_spelling.get(ver, toxenv_for_version(ver))
    for ver in new_versions
]

This way the state is local and Version doesn't need to be extended with complications that affect equality comparisons.

UPDATE: this doesn't consider keeping a consistent style for newly-added versions, for which I'd do

old_spelling = {}
use_dots = False
for env in parse_envlist(envlist):
    ver = tox_env_to_py_version(env)
    if ver:
        old_spelling[ver] = env
        if f'{ver.major}.{ver.minor}' in env:    # py310-django2.7 does not use dots, py3.10 does
            use_dots = True

new_envs = [
    old_spelling.get(ver, toxenv_for_version(ver, use_dots=use_dots))
    for ver in new_versions
]

and now I'm thinking about py310-numpy3.10 and how that would mistakenly trigger the use_dots=True heuristic and maybe the check should be env.startswith(f'py{ver.major}.{ver.minor}')?

What about pypy? Does tox support things like pypy3.11? Do we need to make tox_env_to_py_version() return a tuple (ver, uses_dots)?

I keep almost reconsidering Version.use_dots, but then I think about comparisons again and, no.

[mgedmin]

I'm more worried about the # Try to preserve braced groups branch. Does that continue to work? Does it support dotted versions? E.g. will

[tox]
envlist = py{3.9,3.10}{,-foo}

be converted to

[tox]
envlist = py{3.9,3.10,3.11}{,-foo}

after a check-python-versions --add 3.11?

[gpongelli]

I see. I would hope that tox is smart enough to consider e.g. py38 and py3.8 factors as equivalent, so the configuration would continue to work despite the misalignment.

I've just done some tests, seems not so smart as you hope:

[tox:tox]
envlist = py3.10

[testenv]
commands =
    py3.10: echo dotted
    py310: echo not-dotted
    echo after

if I run poetry run tox, the message I see on screen are dotted and after . Switching to envlist = py310 I see on screen are not-dotted and after .

[mgedmin]

Instead of global state this should take an argument

def toxenv_for_version(ver: Version, use_dots: bool = False) -> str:

[mgedmin]

The regexp should always allow an optional dot. There's no reason to do that only if you know that some of the env names use it.

I think r'py(py)?(\d[.])?\d*($|-)' would work.

(I usually use [.] instead of \. to match literal dots, but that's a personal quirk. There's no real reason to prefer it in this context.)

[mgedmin]

We may have reached the point where manual substring checks are no longer simpler than a regular expression.

Besides, I think this mishandles corner cases like py.29 (should return None) or py10.20 (should return Version('10.20')).

[gpongelli]

removed global variable in favor of an attribute into Version.

Actual solution keeps the dot in envs that have it, and do not add dot to them that haven't it.

[mgedmin]

I'm sorry if I gave the mistaken impression that I want to preserve mixed-style envlists. I don't. I just didn't want a global variable.

What I want is the update_tox_ini_python_versions(), or, actually, update_tox_envlist() to check whether the existing envlist uses dots or not, and then format new_envs accordingly.

[mgedmin]

This doesn't belong here. It has nothing to do with what a Version represents.

[mgedmin]

Why is this needed? Is Version(3, 11, has_dot=True) != Version(3, 11, has_dot=False)? No, that's going to be causing confusion and bugs.

[gpongelli]

I wait answer about this comment before doing anything.

[mgedmin]

#39 (comment) has my latest thoughts (and there's an update where I edited the comment -- IIRC GitHub doesn't sent notification emails for comment edits.)