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

Fix python_requires #3104

Merged
merged 3 commits into from
Jan 24, 2021
Merged

Fix python_requires #3104

merged 3 commits into from
Jan 24, 2021

Conversation

IAlibay
Copy link
Member

@IAlibay IAlibay commented Jan 23, 2021

Fixes #3103

Changes made in this Pull Request:

  • Fixes python_requires in setup.py by specifying the minor version
  • Also sets the max python version to <3.9 (we aren't testing 3.9 for master right now, so if we release it, it's best we avoid allowing users to install 3.9+).

To do

For some reason we don't let 3.5 on master anymore. I was under the impression that we still allowed py3.5 on 1.0.x. I think @orbeckst made the change (from the blame history)?

PR Checklist

  • Tests?
  • Docs?
  • CHANGELOG updated?
  • Issue raised/referenced?

@pep8speaks
Copy link

pep8speaks commented Jan 23, 2021

Hello @IAlibay! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2021-01-24 00:01:03 UTC

@codecov
Copy link

codecov bot commented Jan 23, 2021

Codecov Report

Merging #3104 (57da1f7) into master (658a73d) will increase coverage by 0.93%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3104      +/-   ##
==========================================
+ Coverage   90.89%   91.82%   +0.93%     
==========================================
  Files         149      154       +5     
  Lines       20694    21199     +505     
  Branches     3194     3194              
==========================================
+ Hits        18809    19466     +657     
- Misses       1250     1641     +391     
+ Partials      635       92     -543     
Impacted Files Coverage Δ
package/MDAnalysis/topology/tpr/obj.py 96.82% <0.00%> (-3.18%) ⬇️
...onality_reduction/DimensionalityReductionMethod.py 97.14% <0.00%> (-2.86%) ⬇️
package/MDAnalysis/analysis/legacy/x3dna.py 0.00% <0.00%> (ø)
package/MDAnalysis/due.py 56.09% <0.00%> (ø)
package/MDAnalysis/tests/__init__.py 100.00% <0.00%> (ø)
package/MDAnalysis/analysis/legacy/__init__.py 0.00% <0.00%> (ø)
package/MDAnalysis/tests/datafiles.py 31.25% <0.00%> (ø)
package/MDAnalysis/core/selection.py 99.52% <0.00%> (+<0.01%) ⬆️
...sis/analysis/encore/clustering/ClusteringMethod.py 96.96% <0.00%> (+0.04%) ⬆️
package/MDAnalysis/analysis/base.py 95.60% <0.00%> (+0.04%) ⬆️
... and 85 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 658a73d...57da1f7. Read the comment docs.

@orbeckst
Copy link
Member

I don't remember why I removed 3.5 in PR #2738. You are correct in that I shouldn't have done it because for 1.0.1 we need to support whatever 1.0.0 supported, and in 1.0.0 we did not remove support; in 0.20.0 we removed support for >2.7 <3.5.

The metadata also still showed support for 3.5.

Thanks for catching my mistake.

Copy link
Member

@orbeckst orbeckst left a comment

Choose a reason for hiding this comment

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

👍

@IAlibay
Copy link
Member Author

IAlibay commented Jan 23, 2021

Interesting something about this change is causing py3.5 to break 😕

@orbeckst
Copy link
Member

The pip version is not up to it (from https://travis-ci.com/github/MDAnalysis/mdanalysis/jobs/474702826)

  File "/home/travis/miniconda/envs/test/lib/python3.5/runpy.py", line 193, in _run_module_as_main

    "__main__", mod_spec)

  File "/home/travis/miniconda/envs/test/lib/python3.5/runpy.py", line 85, in _run_code

    exec(code, run_globals)

  File "/home/travis/miniconda/envs/test/lib/python3.5/site-packages/pip/__main__.py", line 21, in <module>

    from pip._internal.cli.main import main as _main

  File "/home/travis/miniconda/envs/test/lib/python3.5/site-packages/pip/_internal/cli/main.py", line 60

    sys.stderr.write(f"ERROR: {exc}")

                                   ^

SyntaxError: invalid syntax

A f-string is used in this pip version.

I wonder why does conda install a pip version that is incompatible with the python version?

(Maybe I ran into the same thing before and couldn't take it anymore and upped the version...)

@IAlibay
Copy link
Member Author

IAlibay commented Jan 23, 2021

Ooo that's really strange, it was running 20.3 just this morning when I re-ran one of the failed jobs, but it's picking up 21 now...

😫 yeah looks like they released 21 just 9h ago..

Related issue: pypa/pip#9500

@orbeckst
Copy link
Member

I sometimes deeply despise new software...

@IAlibay
Copy link
Member Author

IAlibay commented Jan 24, 2021

@orbeckst New release bugs are just a good way keep us on our toes (and burn off HPC allocations) 🙃

Pinning on CI seems to do the trick here. I'm assuming upstream will yank/amend accordingly, so it hopefully shouldn't impact 1.0.1.

py2.7 CI stalled again 🤯 I've restarted it so hopefully that completes....

@IAlibay
Copy link
Member Author

IAlibay commented Jan 24, 2021

py2.7 CI is very flaky, had to keep restarting until it finally worked. I don't think there's much we can do here since we're at a point where py2.7 conda installs is a long mess of frozen solves :/ If someone thinks it's relevant please do open an issue though

@IAlibay IAlibay merged commit 6eb715a into MDAnalysis:master Jan 24, 2021
@IAlibay IAlibay deleted the master-max3.8 branch January 24, 2021 02:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants