-
Notifications
You must be signed in to change notification settings - Fork 0
Fix license collection with the latest pip
packages
#41
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks pretty good to me.
I left a few small questions/suggestions, but nothing critical.
honk
conanfile.py
Outdated
@@ -27,7 +27,7 @@ class EmbeddedPython(ConanFile): | |||
default_options = { | |||
"packages": None, | |||
"pip_version": "24.0", | |||
"pip_licenses_version": "4.4.0", | |||
"pip_licenses_version": "1.4.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't we rename the option to reflect the underlying library?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indeed, renamed and I've noted it in the changelog.
test_package/numpy/test.py
Outdated
extra_argv=["-n", "auto", "-k=not test_mem_policy and not f2py"], | ||
) | ||
) | ||
# `--ignore-glob` expands patterns as absolute paths in the current folder |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't get what this comment is referring to.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, the comment wasn't very helpful. I've expanded it with more info. This also made me realize that the chdir()
I had there originally (Path.home()
) was too broad. The Python prefix path is where numpy
is actually installed in the embedded environment.
# pytest's `--ignore-glob` option (see below) expands patterns as absolute paths in the
# current folder. By default, that's our project folder, but we want to ignore some numpy
# tests so we must target the Python prefix instead.
os.chdir(sys.prefix)
The
pip-licenses
library that we've been using to collect licenses hasn't been maintained in a long time and is falling behind on compatibility: raimon49/pip-licenses#227 The maintained fork/successor ispip-licenses-cli
. It's a drop-in replacement so this is an easy fix.This PR also does some general maintenance. See the individual commits and changelog entries for more details.