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

Transition from setup.py to pyproject.toml #199

Merged
merged 1 commit into from
Mar 15, 2024
Merged

Conversation

wshanks
Copy link
Collaborator

@wshanks wshanks commented Mar 10, 2024

  • Move static package configuration from setup.py to pyproject.toml
    • Minimal changes were made to the configuration, so it still lists Python 2.7 as the minimum version, though it hasn't been tested what the minimum version of Python is that can install the package with a pyproject.toml file and no setup.py.
    • The future dependency was restricted to python < 3.1' since it is not needed with Python 3 and is not actively maintained.
  • Merge long description content in setup.py into README.rst
  • Capture development dependencies listed in setup.py in requirements-dev.txt
  • Save change log which was in long description in setup.py in its own file, CHANGES.rst.

@wshanks
Copy link
Collaborator Author

wshanks commented Mar 10, 2024

This incorporates the change I had proposed in #168.

pyproject.toml Outdated
"Operating System :: OS Independent",
"Programming Language :: Python",
"Programming Language :: Python :: 3",
"Programming Language :: Python :: 3.1",
Copy link
Contributor

@andrewgsavage andrewgsavage Mar 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should python <3.8 be included here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the development plans discussions lists

  1. Support Python 3.8 to 3.12

So you could remove the future dependency and early python versions here, or that can be done in another PR.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, it seemed like there was agreement to move to 3.8+. I started to do that here but then thought I would do it in a separate PR since it is a consequential change and it would be nice to have its own PR for it for someone to comment on. I think we can just make a quick follow up to drop the lines below 3.8.

There is also the question of requires-python. pip obeys that line, so we may not want to set it to >=3.8 if we want to allow Python 3.6 until we intentionally break compatibility, even if we are not actively testing it. I left it at 2.7 here, but I think this change breaks the ability to install the package in Python 2 already since I don't think the last versions of pip and setuptools for Python 2 can install a project using pyproject.toml.

Maybe I should make a PR to drop Python 2 and early Python 3 to merge before this PR so there is a clear progression. I did this part first because I thought it was easier (not needing decide what line to draw with 3.6/3.8).

Copy link
Contributor

@jagerber48 jagerber48 Mar 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I just quickly researched how requires-python works here. It looks like if we set requires-python to >=3.8 then if someone did pip install uncertainties while having Python 3.7 then pip/pypi would NOT let them install the newest version of uncertainties, but it would walk backwards finding older versions until one did not have 3.7 excluded by requires-python.

I think I agree with you we shouldn't set it until we know we've broken compatibility. But I guess I would suggest that we try to set it exactly when we do know we break compatibility. That is, if we know the code is broken for Python 3.7 we should set requires-python = ">=3.8" at that time. That way users hit the error as soon as possible (i.e. at install time). I guess what would happen for that user is pip/pypi would resolve an old enough version of uncertainties that is compatible with their python version. This combined with messaging in the docs about which versions of uncertainties support which versions of Python seems like enough notification to users about versioning considerations.

@wshanks
Copy link
Collaborator Author

wshanks commented Mar 10, 2024

Okay, I made a separate PR (#200) about updating the supported versions of Python and then rebased this one on that. So we can agree there on how to specify Python versions and then make this change. In that PR, I left some opening for continuing to work with older Python versions, but I think this change will preclude easily installing the package with Python 2.

fast calculation of derivatives').\
"""
readme = "README.rst"
requires-python = ">=3.1"
Copy link
Collaborator Author

@wshanks wshanks Mar 10, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because I think this change to pyproject.toml precludes easily installing the package with Python 2 (I don't think there is pyproject.toml support in the libraries left in Python 2 though maybe there are forks maintained somewhere with support for it), I set requires-python>=3.1. pip respects this value so I didn't want to set it too high, so for example someone could install the package with Python 3.6 even though we only test 3.8+. Alternatively, we could just remove this line.

@jagerber48
Copy link
Contributor

I wonder if we should actually release #200 prior to merging in this PR to master. That way there is one versioned release that announces dropping support for earlier versions of python and then a different release that actually drops some versions.

@newville
Copy link
Member

@wshanks @jagerber48 I would be opposed to trying to support Python below 3.8 or claiming that there is support for Python versions that are past their end of life.

We could probably have a setup.py of

import setuptools
setuptools.setup()

I am not going to be able to keep up with multiple PRs and multiple discussion topics per week, especially multiple PRs over a weekend.

@wshanks
Copy link
Collaborator Author

wshanks commented Mar 11, 2024

@wshanks @jagerber48 I would be opposed to trying to support Python below 3.8 or claiming that there is support for Python versions that are past their end of life.

I put some thoughts in #200 (comment). I am open to whatever in this regard. For me, it's easier just to support officially supported versions, but if there is demand I don't see anything strongly pushing for dropping specific versions.

We could probably have a setup.py of

import setuptools
setuptools.setup()

I wondered if that would help with anything but wasn't sure. Maybe if something downstream repackaging the project uses python setup.py still instead of python -m build or pip build?

I am not going to be able to keep up with multiple PRs and multiple discussion topics per week, especially multiple PRs over a weekend.

Sorry -- I am just trying to keep momentum going on the clean up of the project, but there is no rush to get anything in on my part. The only potential pressure is too many conflicting PRs building up unmerged.

* Move static package configuration from setup.py to pyproject.toml
  - Minimal changes were made to the configuration, so it still lists
    Python 2.7 as the minimum version, though it hasn't been tested what
the minimum version of Python is that can install the package with a
pyproject.toml file and no setup.py.
  - Drop the `future` dependency since it is not needed with Python 3.
* Merge long description content in setup.py into README.rst
* Capture development dependencies listed in setup.py in
  requirements-dev.txt
* Save change log which was in long description in setup.py in its own
  file, CHANGES.rst.
Copy link

codecov bot commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

❗ No coverage uploaded for pull request base (master@642c503). Click here to learn what that means.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #199   +/-   ##
=========================================
  Coverage          ?   95.49%           
=========================================
  Files             ?       17           
  Lines             ?     2085           
  Branches          ?        0           
=========================================
  Hits              ?     1991           
  Misses            ?       94           
  Partials          ?        0           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@wshanks
Copy link
Collaborator Author

wshanks commented Mar 15, 2024

With #200 merged, I rebased this on master since I had previously based it on #200's branch.

We can merge it whenever we are ready to break Python 2 compatibility.

I didn't add a setup.py file. My understanding is that running python setup.py has been deprecated for several years now. setup.py is still invoked for more complicated projects that need to set metadata dynamically, but we don't need that here currently. We could still add it if someone thinks it is important.

@newville
Copy link
Member

@wshanks OK, great, and thanks. I think we may not be done with all of this, but let's end Python 2 support!!

@newville newville merged commit e9b82f0 into lmfit:master Mar 15, 2024
18 checks passed
newville added a commit that referenced this pull request Apr 1, 2024
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.

4 participants