Skip to content

BLD: certifi is not a run-time dependency #18636

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 2 commits into from
Oct 2, 2020

Conversation

tacaswell
Copy link
Member

PR Summary

Closes #18337

@tacaswell tacaswell added this to the v3.3.3 milestone Oct 2, 2020
@QuLogic
Copy link
Member

QuLogic commented Oct 2, 2020

I don't remember exactly, but I think I added certifi to runtime because of the download-from-URL style handling. Since this only affected wheel builders, I don't think you'll see whether that's broken from our usual CI.

@tacaswell
Copy link
Member Author

We shouldn't let our build details leak into user space though. If the issue is just our wheel builders we should fix it there by pre-installing like we do with numpy.

I am very hesitant to do it now (because the space is still moving really fast), but eventually we are going to have to have a look at pep517 and friends....

@tacaswell
Copy link
Member Author

now this and ##18637 are going to clash, but will deal with that when it happens.

@tacaswell
Copy link
Member Author

This one should be merged first to make the backport easier.

@QuLogic QuLogic merged commit 80c40b8 into matplotlib:master Oct 2, 2020
meeseeksmachine pushed a commit to meeseeksmachine/matplotlib that referenced this pull request Oct 2, 2020
QuLogic added a commit that referenced this pull request Oct 3, 2020
…636-on-v3.3.x

Backport PR #18636 on branch v3.3.x (BLD: certifi is not a run-time dependency)
@anntzer
Copy link
Contributor

anntzer commented Oct 3, 2020

Actually it is a runtime dependency, e.g. imread("https://matplotlib.org/1.5.0/_static/logo2.png") now requires certifi to be present. In fact I think that 1) this should not actually depend on certifi and 2) we should not support directly opening urls anyways...

@tacaswell tacaswell deleted the bld_intall_dep_certifi branch October 5, 2020 21:47
tacaswell added a commit to tacaswell/matplotlib that referenced this pull request Oct 6, 2020
In matplotlib#18636 we removed certifi as a runtime dependency, but it turns out
we do try to use it if you read from a url in a handful of places.

This makes us fail gracefully and log if we can not import certifi.
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.

Error installing matplotlib-3.3.1 using pip due to old version of certifi on conda environment
4 participants