Skip to content

Convert extras names to lowercase #342

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

Merged
merged 1 commit into from
Mar 4, 2020
Merged

Convert extras names to lowercase #342

merged 1 commit into from
Mar 4, 2020

Conversation

ryan-copperleaf
Copy link
Contributor

Description

Converts the extra names in the package's extras_require to lowercase, working around pypa/pip#4617.

Motivation and Context

For example, even with the latest pip, pip install django-setup-tools[SMS] will not install twilio, even though it should.

How Has This Been Tested?

Tested by installing .[sms], and by confirming that installing .[SMS] continues to do what it did before.

Screenshots (if appropriate):

N/A

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • My code follows the code style of this project.
  • My change requires a change to the documentation. (Only phonenumbers and phonenumberslite seem to be mentioned, which are already lowercase.)
  • I have updated the documentation accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

to work around pypa/pip#4617.

For example, even with the latest pip, `pip install django-setup-tools[SMS]` will not install `twilio`, even though it should.
@codecov
Copy link

codecov bot commented Mar 3, 2020

Codecov Report

Merging #342 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #342   +/-   ##
=======================================
  Coverage   96.70%   96.70%           
=======================================
  Files          39       39           
  Lines        1669     1669           
  Branches      114      114           
=======================================
  Hits         1614     1614           
  Misses         34       34           
  Partials       21       21           

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 ccc11e7...6dc4166. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Mar 3, 2020

Codecov Report

Merging #342 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #342   +/-   ##
=======================================
  Coverage   96.70%   96.70%           
=======================================
  Files          39       39           
  Lines        1669     1669           
  Branches      114      114           
=======================================
  Hits         1614     1614           
  Misses         34       34           
  Partials       21       21           

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 ccc11e7...6dc4166. Read the comment docs.

Copy link
Collaborator

@moggers87 moggers87 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 to me!

I was going to moan about the lack of documentation, but it turns out we don't even document these 🤷‍♀️

@moggers87
Copy link
Collaborator

Just an FYI for those who are also affected by this: it seems like this is a wheel bug as specifying --no-binary :all: with pip or using easy_install solves the issue. Both of those methods treat extras_require keys in a case-insensitive way as expected (I tested this on pip 19.1.1 and easy_install from setuptools 41.2.0)

@moggers87 moggers87 merged commit 3d8147e into jazzband:master Mar 4, 2020
@ryan-copperleaf ryan-copperleaf deleted the patch-1 branch March 5, 2020 23:14
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.

2 participants