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

Relicense from LGPL-3 to BSD-3 #2285

Merged
merged 4 commits into from
Jan 29, 2024
Merged

Conversation

lbdreyer
Copy link
Member

Closes #2134

It has now passed the deadline set in #2134, so we can go ahead with relicensing.

This PR removes all mentions of LGPL and replace it with BSD.

Notably I have had to remove the GSHHS shapefiles that were included in cartopy as these are licensed as LGPL (according to this comment:

gshhs is LGPL, but their documentation doesn't state a version https://www.soest.hawaii.edu/pwessel/gshhg/.

So if we kept the files, the whole project would need to be kept as LGPL.

The files were initially included in this commit: 9fc50a1. They were included to speed up the tests. I will let the tests run to see if the slowdown is significant enough to consider looking at other options.

@lbdreyer
Copy link
Member Author

There is already a fair amount of variability in the time it takes for the tests to run:
image
So the impact of removing those GSHHS files, doesn't appear to have had such a negative affect.

Of course this does open us to the risk of the url we use to download being occasionally slow (or not available) but I suspect there are other parts of Cartopy's testing that requires download

@bjlittle
Copy link
Member

bjlittle commented Nov 13, 2023

@lbdreyer I guess this Note now no longer applies in the cartopy.io.shapereader.GSHHSShpDownloader.acquire_resource.

See here.

@rcomer
Copy link
Member

rcomer commented Nov 13, 2023

Is it deliberate that the LICENSE file says Copyright Met Office but the python file headers say Copyright Cartopy Contributors?

Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

I'd have a slight preference for removing all of the licensing in every file rather than updating it with the new information. Is that an option? It looks like Iris has it in every file too, so if that is required no problem either.

Then we could get rid of the coding standards license test which was not even being run on CI previously: #2278

README.md Outdated Show resolved Hide resolved
docs/source/index.rst Outdated Show resolved Hide resolved
docs/source/copyright.rst Outdated Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
Copy link
Member Author

@lbdreyer lbdreyer left a comment

Choose a reason for hiding this comment

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

Is it deliberate that the LICENSE file says Copyright Met Office but the python file headers say Copyright Cartopy Contributors?

I checked this with the legal team and their recommendation was to include both, i.e. Crown and Cartopy contributors in each copyright reference. So I have updated all of these.

This is a new license, so does the copyright year also need to be update

The year in the copyright notice states the year that the work was first published so this would remain 2012. However, it is not a legal requirement to include it so I have removed it from the copyright notices. We have previously done something similar with the license headers

I'd have a slight preference for removing all of the licensing in every file rather than updating it with the new information. Is that an option?

I'm afraid the legal team recommend we keep these headers

pyproject.toml Show resolved Hide resolved
Copy link
Contributor

@greglucas greglucas left a comment

Choose a reason for hiding this comment

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

Thanks for looking into all of that @lbdreyer! This looks good to me.

docs/source/copyright.rst Show resolved Hide resolved
@greglucas
Copy link
Contributor

This would probably be good to have someone from the Met Office merge in. I think it is good to go, but it would be good to have someone else from the Met Office verify/approve this kind of a change.

@trexfeathers
Copy link
Contributor

This would probably be good to have someone from the Met Office merge in. I think it is good to go, but it would be good to have someone else from the Met Office verify/approve this kind of a change.

Happy to take a look in early January. I've only got a couple of pre-Xmas hours left and I wouldn't want to get trigger happy on something unfamiliar!

@lbdreyer
Copy link
Member Author

@trexfeathers I have just rebased to resolve the merge conflicts

@trexfeathers
Copy link
Contributor

The new failures appear to be affecting any CI that is run. I just tried re-running a job from another PR (link) and the errors came up there too, having not been there before. This all points to something upstream in a dependency.

@rcomer
Copy link
Member

rcomer commented Jan 11, 2024

Test failures need geopython/OWSLib#769 I think ☹

@trexfeathers
Copy link
Contributor

trexfeathers commented Jan 11, 2024

The already exists failures are not the only failures. There is also one here that is due to name clashes with upload-artifact@v4. So not something I can merge currently.

#2313 fixes

Copy link
Contributor

@trexfeathers trexfeathers left a comment

Choose a reason for hiding this comment

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

I note from #2317 that we are happy merging despite the frustrating OWSlib failures, which seems fair to me.

That was the only thing stopping me, so consider Cartopy BSD-3 licensed ✅

@trexfeathers trexfeathers merged commit 97c6a64 into SciTools:main Jan 29, 2024
6 of 21 checks passed
@lbdreyer
Copy link
Member Author

Thank you for merging @trexfeathers !

@QuLogic QuLogic added this to the Next Release milestone Jan 30, 2024
akrherz added a commit to regro-cf-autotick-bot/cartopy-feedstock that referenced this pull request Apr 11, 2024
Zeitsperre added a commit to Ouranosinc/raven that referenced this pull request May 14, 2024
## Overview

This PR fixes #494 

Changes:

* Upgraded Cartopy to v0.23.0 (BSD License)
* Added `yamllint` and `isort` to development deps and to linting
* Dropped support for Python3.8

## Additional Information

SciTools/cartopy#2285
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.

Re-licensing cartopy from LGPL to BSD-3
6 participants