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

Drop official support for Python versions before 3.8 #200

Merged
merged 3 commits into from
Mar 15, 2024

Conversation

wshanks
Copy link
Collaborator

@wshanks wshanks commented Mar 10, 2024

This change does not modify any code, so the codebase is still actually compatible with Python 2.7 through 3.7, but it gives the project the freedom to make incompatible changes with versions in that range in subsequent commits.

A note in the documentation states that compatibility with earlier versions could be considered if there is user demand.

This change does not modify any code, so the codebase is still actually
compatible with Python 2.7 through 3.7, but it gives the project the
freedom to make incompatible changes with versions in that range in
subsequent commits.

A note in the documentation states that compatibility with earlier
versions could be considered if there is user demand.
doc/index.rst Outdated
Supported Python versions
-------------------------

The :mod:`uncertainties` package supports all versions of Python supported by
Copy link
Contributor

Choose a reason for hiding this comment

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

It's true that at this time we've selected to support 3.8-3.12, largely because those are the version currently supported by python. However, I don't think the intention was to tie ourselves to always supporting the same version as Python. On the contrary I think we should support old versions until it becomes costly/annoying to do so for some reason or another.

I think it would suffice to say

The :mod:uncertainties package supports Python 3.8 - 3.12. Supporting earlier Python versions will be considered if there is user demand. Versions of the :mod:uncertainties package up through 3.1.7 supported Python 2.7 through Python 3.12.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Okay, I am fine with changing the text like that. Maybe we should confirm that we want to state 3.8+ here? Syntax-wise the code still supports 2.7+ but with #199 it would become difficult to package and install in 2.7, so I would like to at least officially drop 2.7, which is why I made this separate PR to clarify how we state Python version support (there is also some code clean up that can done after dropping 2.7 which I didn't start on here). I think the relevant reasons for bumping are:

  • 3.0-3.4: I don't think 3.0-3.3 were widely used, so jumping to 3.4 seems okay to me after dropping 2.7.
  • 3.5: aysnc/await syntax, probably not relevant
  • 3.6: f-strings, which are nice for development
  • 3.7: from __future__ import annotations, which converts type annotations to strings so syntax from later Python verisons can be used in type annotations -- nice for adding type annotations
  • 3.8: assignment expressions (:=); can be useful but more something to use when available rather than something to break backwards compatibility for
  • 3.9: Nothing that I know of
  • 3.10: pattern matching (match statement) -- similar to := where it is useful but not a big impact that requires breaking compatibility.
  • 3.11-3.12: Nothing that I know of

So syntax-wise maybe f-strings and type annotations are reason enough to jump to 3.7 at least.

In terms of dependencies, there are only numpy and pytest currently, and there could be deprecations in the standard library modules (mainly math and inspect for uncertainties).

Overall though, no individual features are really compelling to me for dropping support for older versions, except maybe type annotations and 3.7. It is more just that the newer versions add some nice-to-haves and over time the user base of the older versions gets very small and makes it seem not worth avoiding using those newer features.

Copy link
Member

Choose a reason for hiding this comment

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

I vote for testing with only Python 3.8 and later, and saying that we cannot claim that the library works on any system that we do not test.

I would vote for adding an f-string somewhere in the code, perhaps making

return "%r+/-%s" % (self.nominal_value, std_dev_str)
read

        return f"{self.nominal_value}+/-{std_def_err}"

or (but, as with other conversation, maybe this comes later?):

        return f"ufloat({self.nominal_value}, {std_def_err})"

That will deliberately break Python 3.5 and earlier.

I would be okay with saying "We don't test or support Python 3.7, but it might still work for now... until it doesn't.".

@jagerber48
Copy link
Contributor

My recommendation is that the next uncertainties release includes this PR, basically as is. That is:

  • A prominent message in the documentation explaining supported versions of python going forwards (3.8-3.12) and supported versions of python going backwards.
  • Removing unsupported versions from the pypi metadata
  • NO BREAKING CHANGES

So I suggest that the next release does not break anything yet. It seems like we, as a group of new maintainer, should give users at least one release's worth of breathing room where uncertainties will still run on older versions of python despite those versions not being officially supported.

Then the following release allows actually dropping support for the now unsupported python versions, like adding f strings. This would likely be version 4.0.0.

Those are my recommendations for how we drop support for old python versions.

There is a separate question about how we roll out changes which are breaking even for supported versions of python (API/behavior changes, e.g. formatting or others. We should probably have a specific list somewhere of breaking changes we are planning or want to discuss so we can figure out how we want to roll them out.

@newville
Copy link
Member

I stick with my vote of "testing with only Python 3.8 and later, and saying that we cannot claim that the library works on any system that we do not test". I prefer adding f-strings. I would be okay with saying "Python 3.7 or even 3.6 might work, but you're on your own".

Who are the people who a) are using Python 3.5 or earlier, AND b) need to upgrade to the next version of uncertainties? Really, not a rhetorical question: who is that? How would they perform such an upgrade? Note that if we accept #199 as is, then python setup.py install will not work.

I am OK with calling the next version 4.0.0. I see no value in releasing a 3.1.8 that sort of claims support for extremely old versions of Python but does not test with those. Who here is going to support and answer questions about uncertainties from people using Python 3.5 or earlier?

@jagerber48
Copy link
Contributor

I am OK with calling the next version 4.0.0. I see no value in releasing a 3.1.8 that sort of claims support for extremely old versions of Python but does not test with those.

This is fine by me and I even prefer it because it will be easier to implement/support. I guess I was suggesting the conservative strategy in case we wanted to play it safe.

But I agree it's probably a small or zero set of users who would be harmed by dropping support.

So I guess in that case we release 4.0.0 right away. I don't really see a need to force an f string in there to explicitly break Python <=3.5 but I don't really object to it either. If 4.0.0 is the plan then either

  • setup.py looks good in this PR, I'd still like to see my comments about the wording around supported versions in the readme be addressed (that is, I don't think we should suggest that we are pinning uncertainties Python support to the Python support lifecycle. At least we shouldn't suggest that until we've discussed that idea together, which I think we haven't yet. My impression was that we're doing a one time dropping of old versions right now but it's not our plan, e.g. to drop support for the oldest python version each year.

OR

@wshanks
Copy link
Collaborator Author

wshanks commented Mar 12, 2024

I feel similarly to @newville. For simplicity, I would rather just not consider versions before 3.8. If there is demand, I don't mind keeping unofficial support for 3.7. I don't see any big reason to block 3.7 and am willing to rework a change that breaks 3.7 if there is a simple and compatible alternative, but I would rather keep it at that low level where we fix something after someone notices and reports it rather than continuing to test. Also, I would like to keep the option of making a hard break when necessary without an additional deprecation cycle.

My choice of 3.8 is based on it being the oldest supported version currently. If it were up to me, I would similarly deprecate 3.8 some time after 3.13 comes out, meaning again not a hard break, but just not paying attention to 3.8 and being open to breaking compatibility if there were a reason. I don't feel strongly though and we don't need to commit to something now so I removed the wording in the documentation. Since most projects got over the Python 2 transition, I find that many major projects (numpy, scipy, pandas, django, pytest, etc) follow the supported Python versions aggressively, even dropping support before upstream Python does.

My preference is to merge this before #199 because I think changing the supported Python versions is a big enough change for one PR (even if only a nominal change). I don't think we need a release just to signal to users that the project is active again but releasing something soon with some fixes would be nice for that. I opened #201 for discussing upcoming releases.

@newville
Copy link
Member

@wshanks Thanks -- I agree with all of that.

@andrewgsavage
Copy link
Contributor

I agree too, can this be merged?

@jagerber48
Copy link
Contributor

Well if everyone agrees that we will pin uncertainties testing to the python lifecycle then it would be ok to include that language back in the readme. I just flagged it since I feel we hadn't yet discussed it in detail. But anyways, I'd be happy with it merged in as is. Like has been said above, I don't see reason to explicitly make uncertainties break on python versions that aren't supported, rather we should just pull them out of CI and PyPi metadata and not take any pains to maintain backwards compatibility with those versions.

That said, if we do know that uncertainties breaks on a certain unsupported versions of python then I do think we should include requires-python to exclude that version in pyproject.toml. So, e.g., when we put in the first f-string we should set requires-python = ">=3.6".

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@05361a7). Click here to learn what that means.

Additional details and impacted files
@@            Coverage Diff            @@
##             master     #200   +/-   ##
=========================================
  Coverage          ?   94.94%           
=========================================
  Files             ?       18           
  Lines             ?     2097           
  Branches          ?        0           
=========================================
  Hits              ?     1991           
  Misses            ?      106           
  Partials          ?        0           

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

@newville
Copy link
Member

@wshanks I believe that the suggestion is still to merge this before #199.... hoping that is correct, I will squash and merge, and then #199 later today.

@newville newville merged commit 642c503 into lmfit:master Mar 15, 2024
18 checks passed
@wshanks wshanks deleted the py2 branch March 15, 2024 16:17
@wshanks
Copy link
Collaborator Author

wshanks commented Mar 15, 2024

@newville Yes, that's great -- sorry I have been behind on things this week!

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.

None yet

4 participants