The Wayback Machine - https://web.archive.org/web/20220526061216/https://github.com/scikit-learn/scikit-learn/pull/23451
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

Jump to bottom

Add BSD license trove classifier #23451

Merged
merged 3 commits into from May 24, 2022
Merged

Add BSD license trove classifier #23451

merged 3 commits into from May 24, 2022

Conversation

johnthagen
Copy link
Contributor

@johnthagen johnthagen commented May 24, 2022 •
edited

Add the BSD license trove classifier. This helps license report generators such as pip-licenses generate standard license reports.

This also matches scikit-image:

ogrisel
ogrisel approved these changes May 24, 2022
View changes
Copy link
Member

@ogrisel ogrisel left a comment

I don't know if it's possible / useful / needed to be more specific and mention the specific BSD variant in the trove, but this is already a net improvement as such.

I think we could remove the line 257 with:

        "License :: OSI Approved",

or alternatively group the two lines together.

Otherwise LGTM.

@lesteve
Copy link
Member

@lesteve lesteve commented May 24, 2022

In my last commit I used only a single-line for the licence info as advised by @ogrisel

@lesteve
Copy link
Member

@lesteve lesteve commented May 24, 2022 •
edited

The doc failure in my commit does not make sense to you, does it for you @thomasjpfan? Is it a known limitation of build_tools/circle/checkout_merge_commit.sh somehow?

From the build log it seems like some changes from main are in the branch (e.g. it is using conda-lock for doc builds) but not all (LOCK_FILE env var is not defined). Edit: wild-guess maybe the .circleci/config.yml is always taken from the PR branch

Maybe not worth investigating since pulling main fixes it but weird nonetheless ...

@johnthagen
Copy link
Contributor Author

@johnthagen johnthagen commented May 24, 2022 •
edited

I think we could remove the line 257 with:

Yes, I should have done that to begin with. I overlooked that a less specific classifier existed.

Thank you for fixing that.

@lesteve
Copy link
Member

@lesteve lesteve commented May 24, 2022 •
edited

I check pip-licenses on this branch it gives this (same thing as scikit-image):

❯ pip-licenses | grep scikit
 scikit-image                   0.19.2     BSD License                                        
 scikit-learn                   1.2.dev0   BSD License

Merging, thanks!

@lesteve lesteve merged commit 996c7e2 into scikit-learn:main May 24, 2022
30 checks passed
@thomasjpfan
Copy link
Member

@thomasjpfan thomasjpfan commented May 24, 2022

circleci usually fails when we change something about it.

wild-guess maybe the .circleci/config.yml is always taken from the PR branch

That is my guess as well.

@johnthagen johnthagen deleted the patch-1 branch May 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel

@thomasjpfan thomasjpfan

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
4 participants
@johnthagen @lesteve @thomasjpfan @ogrisel