-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Increase python requirement to >= 3.8 #2029
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
Changes from all commits
c1f37c5
4461abf
58136fa
584275c
f428ccd
942d83f
ae97161
6582f81
dde1bae
6139435
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
This file was deleted.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,8 +103,7 @@ | |
|
||
tz = 'Asia/Calcutta' | ||
lat, lon = 28.6, 77.2 | ||
times = pd.date_range('2019-01-01 00:00:00', '2020-01-01', closed='left', | ||
freq='H', tz=tz) | ||
times = pd.date_range('2019-01-01 00:00:00', '2020-01-01', freq='H', tz=tz) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Did this break with python 3.8 even though the pandas version didn't change? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Prior to this PR, we had ReadTheDocs configured to use python 3.7, but we did not specify a particular pandas version to use for the documentation build. That means RTD had been using the latest version of pandas that was available for python 3.7 (pandas 1.3.5). With this PR updating the configuration to use python 3.8 but leaving the pandas version unspecified, ReadTheDocs then had access to a newer version of pandas, and in particular one that required these changes (pandas 2.0.3). So updating the python version used in the docs build did change the pandas version that gets used, since we aren't telling RTD to use a particular pandas version. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Gotcha. So the equivalent would now be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, I think so. And I agree it does not matter for this example, so I opted to simplify and just remove the parameter rather than update to the new name. |
||
|
||
solpos = solarposition.get_solarposition(times, lat, lon) | ||
# remove nighttime | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Is this change going to affect a patch version or a minor one, v0.11.0, is preferred? I expected next release to be v0.11 but dunno. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree there is reason to generally prefer to drop support for a python version in a minor release, not a patch release. The choice to do it in a patch release in this particular case was motivated by (taken from #1975 (comment)):
So we want to make a release soon, but we still want to wait a little while for 0.11.0. I think including this in 0.10.5 is the best option. Also, this won't be the first time that we have dropped support for a python version in a patch release. For example we dropped 3.6 in v0.9.2. So I don't love it either, but I don't see a better option. Ideas are welcome :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That's fine. I don't have any strong opinion other than NEP29, but since this project doesn't follow it, I only advocate for moving forward the supported python version ^.^ Thanks for the explanation! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I understood the information on pypi correctly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Version 3.7.7.0 has wheels available for python 3.8: https://pypi.org/project/ephem/3.7.7.0/#files
That's how I identified these versions. Is there a different/better way?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're asking the wrong person! I was just browsing to get a sense for the consequences.