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

Modernize the code to Python 3.6+ #155

Closed
wants to merge 5 commits into from
Closed

Modernize the code to Python 3.6+ #155

wants to merge 5 commits into from

Conversation

eumiro
Copy link
Contributor

@eumiro eumiro commented Feb 4, 2021

Now that we're at Python 3.6+, we can modernize the code and add f-strings, pathlib.Path and other Python3 features.

Next steps would be to convert the tests to pytest. What do you think?

@eumiro eumiro mentioned this pull request Feb 4, 2021
@coveralls
Copy link

coveralls commented Feb 4, 2021

Coverage Status

Coverage decreased (-1.0%) to 99.035% when pulling 69f75d0 on eumiro:py36plus into d2c9f71 on jsvine:master.

@jsvine
Copy link
Owner

jsvine commented Feb 6, 2021

Thanks! On the whole, I like the ideas here. But this PR introduces a lot of line-changes, spanning a few different suggestions, so it'll take me a little while to audit it. (I realize that many of the changes are auto-generated, but I'd still like to read them carefully.)

@eumiro
Copy link
Contributor Author

eumiro commented Feb 6, 2021 via email

@jsvine
Copy link
Owner

jsvine commented Feb 19, 2021

I think it's fine to keep this PR as-is, but thanks for offering. I haven't forgotten about the PR, but have been busy. I hope to take a closer look soon.

@jsvine
Copy link
Owner

jsvine commented Jul 16, 2021

Thanks again, @eumiro. I've taken some of these ideas, and some of the commits directly, and incorporated them into #166, now merged.

One thing I didn't do was to use pathlib; it's a good idea, but does not play a major role here. If you'd like, feel free to submit a PR specifically for that.

@jsvine jsvine closed this Jul 16, 2021
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

3 participants