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

Python 3 #121

Merged
merged 8 commits into from
Sep 15, 2019
Merged

Python 3 #121

merged 8 commits into from
Sep 15, 2019

Conversation

weaverba137
Copy link
Member

This PR eliminates all support for Python 2. This does not need to be merged immediately; it is more of an exercise in what we can we do when we drop Python 2 support.

@weaverba137 weaverba137 self-assigned this Sep 20, 2018
@weaverba137 weaverba137 added this to In progress in 18.11 via automation Sep 20, 2018
@sbailey
Copy link
Contributor

sbailey commented Sep 20, 2018

What is the motivation for this update? It appears that it drops support for py2 but I don't see what the actual benefit is. Does it let us do something that wasn't possible before? It's fine if we want to add a new piece of code that for whatever reason only works with py3, but going back and stripping out py2 support feels like a distraction.

@weaverba137
Copy link
Member Author

As I mentioned in the description, this is an exercise. I found quite a bit of code that is completely unnecessary, and might even be confusing in the future, given that we won't be using Python 2 anyway.

In addition, Astropy has taken the proactive step of removing Python-2-compatible code, and I wanted to see what that would look like.

@sbailey
Copy link
Contributor

sbailey commented Sep 24, 2018

OK as an exercise, but please don't spend the time to do this for every other repo right now. I'm also inclined to not merge this in desiutil right now, because it is at the bottom of our dependency chain and if desiutil breaks py2, then effectively that breaks py2 for everything. We don't need to spend effort maintaining py2 if some new piece of code happens to not work with it, but let's not spend our time purposefully breaking things that still work. OK to leave branch open for merging someday, though perhaps closing the PR (at minimum I'm going to remove it from the 18.9 critical path).

@sbailey sbailey removed this from In progress in 18.11 Sep 24, 2018
@weaverba137
Copy link
Member Author

Agreed. In the meantime, I'm going to remove .gitlab-ci.yml from master.

@weaverba137
Copy link
Member Author

@sbailey , could you revisit this when you get a chance? This has been hanging for almost a year. At this point, I think anyone still trying to use desiutil with Python 2 should be forced to upgrade.

Copy link
Contributor

@sbailey sbailey left a comment

Choose a reason for hiding this comment

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

OK to merge when ready. Onward with python 3.

@weaverba137
Copy link
Member Author

Thank you!

@weaverba137 weaverba137 merged commit e04de10 into master Sep 15, 2019
@weaverba137 weaverba137 deleted the python-3 branch September 15, 2019 23:21
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

2 participants