Skip to content

Don't pretend to support old pythons in setup.py. #16837

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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

Don't pretend to support old pythons in setup.py. #16837

wants to merge 1 commit into from
+2 −18

Conversation

anntzer
Copy link
Contributor

@anntzer anntzer commented Mar 19, 2020

setup.py uses f-strings, so it fails with SyntaxError on Py<3.6, so
let's not pretend to support older Pythons there (see also #16733).

(The alternative would be to only put the version check in setup.py and
then move everything else into setupext.py, which may not be silly
either?)

PR Summary

PR Checklist

  • Has Pytest style unit tests
  • Code is Flake 8 compliant
  • New features are documented, with examples if plot related
  • Documentation is sphinx and numpydoc compliant
  • Added an entry to doc/users/next_whats_new/ if major new feature (follow instructions in README.rst there)
  • Documented in doc/api/api_changes.rst if API changed in a backward-incompatible way

@anntzer
Don't pretend to support old pythons in setup.py.
44570e8 
setup.py uses f-strings, so it fails with SyntaxError on Py<3.6, so
let's not pretend to support older Pythons there.

(The alternative would be to only put the version check in setup.py and
then move *everything* else into setupext.py, which may not be silly
either?)
@anntzer anntzer added the Build label Mar 19, 2020
@anntzer anntzer added this to the v3.3.0 milestone Mar 19, 2020
@QuLogic
Copy link
Member

QuLogic commented Mar 19, 2020

If anything, #16733 seems to prove that we need to keep the check in longer. Python 2 was only just let go this year.

@anntzer
Copy link
Contributor Author

anntzer commented Mar 19, 2020

But the point is that the check is actually not doing its job, because if you're running setup.py in an old Python you get the syntax error even before the check runs.

@timhoffm
Copy link
Member

The point is to fix that other syntax error and not to add more incompatibility?

@anntzer
Copy link
Contributor Author

anntzer commented Mar 20, 2020 •
edited
Loading

The point is to fix that other syntax error

This is bound to break regularly again unless we actually test that.
If we care about this use case I would suggest that we just move everything to setupext.py with just a version check in setup.py before importing setupext (which is fine too; do you agree with this plan?).

@timhoffm
Copy link
Member

The jquery stuff and command definitions surely should go into setupext.py; as well as the package collection. However I would leave setup() in place. That's where people expect it.

@QuLogic QuLogic mentioned this pull request Apr 8, 2020
Merged
2 tasks
@QuLogic
Copy link
Member

QuLogic commented Apr 8, 2020

Since we got another report about the SyntaxError, I don't think we're ready to be removing this check. I've opened the above-mentioned PR to fix the syntax errors instead. I'm going to close this for now.

@QuLogic QuLogic closed this Apr 8, 2020
@anntzer anntzer deleted the setuppyver branch April 8, 2020 21:45
@anntzer
Copy link
Contributor Author

anntzer commented Apr 8, 2020

Sounds fair.

@QuLogic QuLogic modified the milestones: v3.3.0, unassigned Jul 9, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
Build
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@anntzer @QuLogic @timhoffm