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

Support Python 3.12 and drop 3.7 #164

Merged
merged 2 commits into from
Oct 22, 2023

Conversation

max-muoto
Copy link
Contributor

@max-muoto max-muoto commented Oct 21, 2023

As mentioned in this issue #163 pkg_resources is deprecated and importlib should be used instead. Putting out this PR as this is blocking our upgrade to Python 3.12

@codecov
Copy link

codecov bot commented Oct 21, 2023

Codecov Report

Merging #164 (ada93da) into master (4d4ef69) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff            @@
##            master      #164   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           11        11           
  Lines          400       400           
  Branches        68        68           
=========================================
  Hits           400       400           
Files Coverage Δ
django_hosts/__init__.py 100.00% <100.00%> (ø)

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

Copy link
Member

@ddabble ddabble left a comment

Choose a reason for hiding this comment

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

Looks good!

@max-muoto max-muoto marked this pull request as ready for review October 21, 2023 22:28
@max-muoto
Copy link
Contributor Author

Before merging this in, one issue is that 3.7 users might not have the importlib_metadata backport installed. Seeing this library doesn't have any external requirements. Are we fine with adding the backport in as a requirement for 3.7 users, or is there any thought around dropping support for 3.7?

@ddabble
Copy link
Member

ddabble commented Oct 21, 2023

Good point! I'm personally planning on opening a PR removing support for Python 3.7, since its EOL has passed (as has been done with previous Python versions), but presuming this PR will be merged before that, we should probably keep the code using pkg_resources for Python versions < 3.7..

Any thoughts? Would you mind making that change? 🙂

@max-muoto
Copy link
Contributor Author

Good point! I'm personally planning on opening a PR removing support for Python 3.7, since its EOL has passed (as has been done with previous Python versions), but presuming this PR will be merged before that, we should probably keep the code using pkg_resources for Python versions < 3.7..

Any thoughts? Would you mind making that change? 🙂

Sure, let me go ahead and package that in with this PR! I'll also add a CI check for 3.12.

@ddabble
Copy link
Member

ddabble commented Oct 21, 2023

I'll also add a CI check for 3.12.

Sure, given that it passes 😅

@max-muoto
Copy link
Contributor Author

I'll also add a CI check for 3.12.

Sure, given that it passes 😅

Went ahead and removed the conditional import since we can now expect that all users will have importlib.metadata and added CI checks if you want to run the pipeline.

@max-muoto
Copy link
Contributor Author

@ddabble Looks like all checks are passing, and for 3.12, any blockers from merging this?

ddabble added a commit to max-muoto/django-hosts that referenced this pull request Oct 22, 2023
Copy link
Member

@ddabble ddabble left a comment

Choose a reason for hiding this comment

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

Hm, it's now moved a little too far out of scope of the original PR, in my opinion, but if we rename it to e.g. "Support Python 3.12 and drop 3.7", it should be good :)

I pushed a few commits to add some changes I think were missing. Do you mind if I reorganize the commits before merging? I was thinking one commit for dropping 3.7 and one for supporting 3.12.

@max-muoto max-muoto changed the title Replace pkg_resources with importlib.metadata Support Python 3.12 and drop 3.7 Oct 22, 2023
@max-muoto
Copy link
Contributor Author

Hm, it's now moved a little too far out of scope of the original PR, in my opinion, but if we rename it to e.g. "Support Python 3.12 and drop 3.7", it should be good :)

Good callout, went ahead and renamed it!

I pushed a few commits to add some changes I think were missing. Do you mind if I reorganize the commits before merging? I was thinking one commit for dropping 3.7 and one for supporting 3.12.

Thanks for doing that! Sure thing, I didn't know if they would get squashed or not, so appreciate it!

@ddabble
Copy link
Member

ddabble commented Oct 22, 2023

Btw, I noticed that you removed testing Django 4.1 with Python 3.12 in 3aea202, but it seems like the tests pass after undoing it in e293a76. Any particular reason..? 🙂

@max-muoto
Copy link
Contributor Author

Btw, I noticed that you removed testing Django 4.1 with Python 3.12 in 3aea202, but it seems like the tests pass after undoing it in e293a76. Any particular reason..? 🙂

When doing that, wasn't 100% sure if it would pass, and saw that official support for 3.12 was only going to be a part of 4.2.

@ddabble
Copy link
Member

ddabble commented Oct 22, 2023

Ah, I see! I guess we should ideally keep your version, then :)

@ddabble ddabble linked an issue Oct 22, 2023 that may be closed by this pull request
Also removed usage of `pkg_resources` in `django_hosts/__init__.py`, as
it's been deprecated (see the "Attention" notice at the top of
https://setuptools.pypa.io/en/latest/pkg_resources.html).
`importlib.metadata.version()` was added in Python 3.8, which is the
new, recommended way to get the version of an installed package.

Co-authored-by: Anders <6058745+ddabble@users.noreply.github.com>
ddabble added a commit to max-muoto/django-hosts that referenced this pull request Oct 22, 2023
Co-authored-by: Anders <6058745+ddabble@users.noreply.github.com>
@ddabble ddabble force-pushed the Remove-usage-of-pkg_resources branch from d87d18a to 48b1fde Compare October 22, 2023 21:53
Co-authored-by: Anders <6058745+ddabble@users.noreply.github.com>
@ddabble ddabble force-pushed the Remove-usage-of-pkg_resources branch from 48b1fde to ada93da Compare October 22, 2023 21:54
Copy link
Member

@ddabble ddabble left a comment

Choose a reason for hiding this comment

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

Alright, thank you! 😊

@ddabble ddabble merged commit 6b6d0a4 into jazzband:master Oct 22, 2023
8 checks passed
ddabble added a commit that referenced this pull request Oct 22, 2023
Also removed usage of `pkg_resources` in `django_hosts/__init__.py`, as
it's been deprecated (see the "Attention" notice at the top of
https://setuptools.pypa.io/en/latest/pkg_resources.html).
`importlib.metadata.version()` was added in Python 3.8, which is the
new, recommended way to get the version of an installed package.

Co-authored-by: Anders <6058745+ddabble@users.noreply.github.com>
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.

DeprecationWarning: pkg_resources is deprecated as an API
2 participants