Skip to content
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

python_requires not written when only --min-py3-version is specified #178

Open
mxr opened this issue Dec 6, 2022 · 9 comments · May be fixed by #179
Open

python_requires not written when only --min-py3-version is specified #178

mxr opened this issue Dec 6, 2022 · 9 comments · May be fixed by #179

Comments

@mxr
Copy link
Sponsor Contributor

mxr commented Dec 6, 2022

See repro:

def test_guess_python_min_version(tmpdir):
    setup_cfg = tmpdir.join('setup.cfg')
    setup_cfg.write(
        '[metadata]\n'
        'name = pkg\n'
        'version = 1.0\n',
    )

    assert main((str(setup_cfg), '--min-py3-version=3.8'))
$ pytest -vvv -k test_guess_python_min_version tests/setup_cfg_fmt_test.py
========================================= test session starts ==========================================
platform darwin -- Python 3.9.15, pytest-6.2.4, py-1.10.0, pluggy-0.13.1 -- /Users/mxr/src/mxr/unkey/venv/bin/python
cachedir: .pytest_cache
rootdir: /Users/mxr/src/asottile/setup-cfg-fmt
collected 67 items / 66 deselected / 1 selected                                                        

tests/setup_cfg_fmt_test.py::test_guess_python_min_version FAILED                                [100%]

=============================================== FAILURES ===============================================
____________________________________ test_guess_python_min_version _____________________________________

tmpdir = local('/private/var/folders/mg/2_7nx7493qg0vn9jhdp951tw0000gp/T/pytest-of-mxr/pytest-10/test_guess_python_min_version0')

    def test_guess_python_min_version(tmpdir):
        setup_cfg = tmpdir.join('setup.cfg')
        setup_cfg.write(
            '[metadata]\n'
            'name = pkg\n'
            'version = 1.0\n',
        )
    
>       assert main((str(setup_cfg), '--min-py3-version=3.8'))
E       AssertionError: assert 0
E        +  where 0 = main(('/private/var/folders/mg/2_7nx7493qg0vn9jhdp951tw0000gp/T/pytest-of-mxr/pytest-10/test_guess_python_min_version0/setup.cfg', '--min-py3-version=3.8'))

tests/setup_cfg_fmt_test.py:667: AssertionError
======================================= short test summary info ========================================
FAILED tests/setup_cfg_fmt_test.py::test_guess_python_min_version - AssertionError: assert 0
=================================== 1 failed, 66 deselected in 0.17s ===================================
@asottile
Copy link
Owner

asottile commented Dec 6, 2022

probably worth fixing -- I think this can be fixed with a None check after trying tox and classifiers and if an explicit minimum version is passed then use that (otherwise do nothing)

@mxr
Copy link
Sponsor Contributor Author

mxr commented Dec 6, 2022

The behavior repros when --min-py3-version is not passed too and a default is used for min_py3_version. So should the fix change the parsing code to determine when absolutely nothing is passed in, or could the fix just set python_requires to whatever the minimum is after ingesting all sources (tox envlist, python_requires in setup.cfg, min_py3_version flag, etc)?

@asottile
Copy link
Owner

asottile commented Dec 6, 2022

yeah probably should do it for the default too -- I guess there'd then be no way to avoid python_requires being set by setup-cfg-fmt -- but I think that's a good thing (everything should set it always)

@mxr
Copy link
Sponsor Contributor Author

mxr commented Dec 8, 2022

Going to try to get a PR for this out this week

@mxr
Copy link
Sponsor Contributor Author

mxr commented Dec 11, 2022

Working on this and the behavior I'm writing is "set python_requires to whatever the minimum is after ingesting all sources (tox envlist, python_requires in setup.cfg, min_py3_version flag, etc)" but I wanted to confirm if this is really the desired behavior. For example the test test_min_version_removes_classifiers would not longer rewrite python_requires, classifiers, etc to be >=3.4 because >=3.2 is already in python_requires. Is that what you want?

Perhaps the right behavior is: if min_py3_version is passed in, use that to determine the min, otherwise, try to guess the min based on all the other sources. That has the convenient behavior of being able to drop/support python versions in the setup.cfg metadata by simply changing the min_py3_version flag

@asottile
Copy link
Owner

yeah the latter probably makes more sense

@mxr
Copy link
Sponsor Contributor Author

mxr commented Dec 11, 2022

So I realized from the test_min_py3_version_updates_python_requires test that min_py3_version should only override python_requires when py27 is not supported in python_requires, which makes the implementation more complicated. Wdyt about min_py3_version being the floor for python_requires even if py27 is listed? (Effectively changing it to min_py_version)

@asottile
Copy link
Owner

I'm fine killing python 2 support if it's easy -- actually probably about time

@mxr
Copy link
Sponsor Contributor Author

mxr commented Dec 11, 2022

sg

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 a pull request may close this issue.

2 participants