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

Update to copyleft license #78

Merged
merged 11 commits into from
Jun 26, 2020
Merged

Conversation

x0s
Copy link
Contributor

@x0s x0s commented Jun 9, 2020

As discussed with the pyronear contributors, this PR aims at licencing our work as free software through CeCILL v2.1 and AGPLv3 licences.

Motivation:
As "free" in freedom, the contributors like their work to stay free in order to encourage open collaboration and to protect the freedom of users to improve the library for common good (at least help preventing wildfires and share knowledge)

Some notes:

  • Dual-licencing with CeCILL enables the user to comply more easily with French law (if needed)
  • To inform the user how the code is distributed, a short header is inserted in every file:
# Copyright (c) Pyronear contributors.
# This file is dual licensed under the terms of the CeCILL-2.1 and AGPLv3 licenses.
# See the LICENSE file in the root of this repository for complete details.

@x0s x0s self-assigned this Jun 9, 2020
Copy link
Contributor

@Akilditu Akilditu left a comment

Choose a reason for hiding this comment

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

Thanks a lot for this PR !

General comments :

  • Is there a smooth and dynamic way to insert the header in every new files ?
  • Following this 1st statement should we create a test to ensure that headers are well incorporated in those new files ?

@codecov
Copy link

codecov bot commented Jun 9, 2020

Codecov Report

Merging #78 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master      #78   +/-   ##
=======================================
  Coverage   85.73%   85.73%           
=======================================
  Files          21       21           
  Lines         820      820           
=======================================
  Hits          703      703           
  Misses        117      117           
Flag Coverage Δ
#unittests 85.73% <ø> (ø)
Impacted Files Coverage Δ
pyronear/datasets/__init__.py 100.00% <ø> (ø)
pyronear/datasets/openfire.py 89.18% <ø> (ø)
pyronear/datasets/utils.py 91.66% <ø> (ø)
pyronear/datasets/wildfire/__init__.py 100.00% <ø> (ø)
pyronear/datasets/wildfire/fire_labeler.py 100.00% <ø> (ø)
pyronear/datasets/wildfire/frame_extractor.py 100.00% <ø> (ø)
pyronear/datasets/wildfire/split_strategy.py 97.29% <ø> (ø)
pyronear/datasets/wildfire/wildfire.py 98.38% <ø> (ø)
pyronear/models/__init__.py 100.00% <ø> (ø)
pyronear/models/densenet.py 58.53% <ø> (ø)
... and 10 more

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 df7bcf6...2ea9dbe. Read the comment docs.

@x0s
Copy link
Contributor Author

x0s commented Jun 9, 2020

Thanks a lot for this PR !

General comments :

* Is there a smooth and dynamic way to insert the header in every new files ?

@Akilditu: Thanks for your comments.
Actually, I found my wrist quite smooth and dynamic when copy-pasting these headers ;D

More seriously, it would be possible to write a script that automatically add these headers. I don't think we are going to add new files so often, so that shouldn't be a big deal to copy-paste it.
Besides, another common practice is to let your favourite IDE add it for you.

* Following this 1st statement should we create a test to ensure that headers are well incorporated in those new files ?

One can imagine parsing all .py files first lines looking for these headers. It could be a good first issue ;)

@Akilditu
Copy link
Contributor

Thanks for you reply !

One can imagine parsing all .py files first lines looking for these headers. It could be a good first issue ;)

Agreed on that point, will create an issue after merge :)

Akilditu
Akilditu previously approved these changes Jun 10, 2020
Copy link
Contributor

@Akilditu Akilditu 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 you precisions, everything is OK on my side !

Copy link
Member

@frgfm frgfm 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 the license update! A small suggestion in the licensing header about the organization name and the rest looks good to me

LICENSE Outdated

Copyright (c) 2019 F-G Fernandez
Copyright (c) 2019-2020 The pyronear developers.
Copy link
Member

Choose a reason for hiding this comment

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

Thanks for updating this!
I looked around a bit and apparently, after the year should come the name of the person or organisation. This seems to add up with the default headers for the GPLv3 at least in the github templates (https://github.com/licenses/license-templates/blob/master/templates/gpl3-header.txt). Perhaps we should do "Copyright (c) 2019-2020 Pyronear", don't you think?

Having pyronear in lowercase and "The" before that made me realise that people might think the organization name is "The pyronear developpers" 😅

Copy link
Contributor Author

@x0s x0s Jun 11, 2020

Choose a reason for hiding this comment

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

If more than one person is involved, why not also mentioning these persons ?
This point was asked at yesterday meeting, everyone seemed to agree that's better to emphasize on the human that produce the code more than the organization behind.
More over, using 'developers' will include any contributor who is not a member of the organization
FYI, it is common to phrase it this way: https://github.com/scikit-learn/scikit-learn/blob/master/COPYING#L3, https://github.com/scipy/scipy/blob/master/LICENSE.txt#L1

Copy link
Member

Choose a reason for hiding this comment

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

Ok, I get your point now. Thanks for the clarification 👍
I would only suggest then "Pyronear contributors" (or "Pyronear developers" if you prefer), the "The" seems a bit off to me weirdly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for asking.
Let's be Pyronear contributors then ! ;)

pyronear/datasets/openfire.py Outdated Show resolved Hide resolved
@fe51 fe51 self-requested a review June 10, 2020 17:41
@2ndcouteau
Copy link

I checked the others GPL licenses and I found the AGPL or also this clear explanation.
It's the same as GPLv3 but with one more restriction/protection for the freedom of the project. It's quite subtle but could be important.
As far as I understand, with the GPLv3, if someone get the project, use it for it's own service but without a form of (direct) distribution, like the Software as a Service (SaaS) model can do, so this person can keep his own improvements for himself.

So with the GPLv3, someone could be able to create a service that will evaluate an image with the Pyronear program and then return the evaluation. OK until here.
Now, if he decides to improve the program and to provide these improvements through his service, he will not have any obligation to publish these improvements.
Because it's a service, it is not considered as a distribution.

It's also a way to keep the big clouds of the world from turning one's project into their product. Small wonder, then, that some companies that license their code under the AGPL internally describe it as the "Amazon GPL." AWS, for example, has made orders of magnitude more money from MySQL than MySQL AB (and now Oracle) ever hoped to make (through RDS). By licensing with the AGPL, these companies ensure that no one besides themselves can monetize the project.

Source

As our position seems to be to keep this project as more as possible as a public property, I suggest to use the AGPLv3 instead of GPLv3.

@blenzi
Copy link

blenzi commented Jun 11, 2020

Thanks @x0s! Just a few comments:

  • I guess we were all puzzled about Copyright (c) The pyronear developers. for a copy-left license. My understanding is that is refers more to Pyronear. Are we sure we can have it?
  • I agree with @Akilditu : including a check for each file would be nice. Since we are still discussing, maybe it is worth adding it already to this PR ?
  • A funny detail: the title of this PR has licence, the file name is LICENSE. Both seem to be accepted, we just need to avoid mixing them up

@frgfm frgfm changed the title Update to copyleft licence Update to copyleft license Jun 11, 2020
@frgfm
Copy link
Member

frgfm commented Jun 11, 2020

@2ndcouteau nice finding, would you have the requirements of the license as well by any chance? (which mentions & files do we have to put in the repo and in the headers). So that we can compare our options here 🙏

@blenzi a few inputs if I may

  • Yes it's possible, there are many examples on GNU website and you can check by yourself with github: when you create a repo, you can choose the license and whether you choose MIT our GPL, the copyright does appear :) Just picture it as an identifier of the authors/contributors
  • I agree with both of you that we may add a workflow for this! But it would be a waste not to let the opportunity to someone else do the PR hehe (happy to help 👌 )
  • Licence is British English while License is for the US, but since we do already have a LICENSE file, I just edited the name of the PR to avoid confusion!

@x0s
Copy link
Contributor Author

x0s commented Jun 11, 2020

Thanks @x0s! Just a few comments:

* I guess we were all puzzled about `Copyright (c) The pyronear developers.` for a copy-left license. My understanding is that is refers more to _Pyronear_. Are we sure we can have it?

* I agree with @Akilditu : including a check for each file would be nice. Since we are still discussing, maybe it is worth adding it already to this PR ?

* A funny detail: the title of this PR has _licence_, the file name is _LICENSE_. Both seem to be accepted, we just need to avoid mixing them up

Hi thanks for the review!
I confused the english and french words, so let it be LICENSE:)
I will push tests to check headers today, I agree this is the right moment

@x0s
Copy link
Contributor Author

x0s commented Jun 11, 2020

@2ndcouteau nice finding, would you have the requirements of the license as well by any chance? (which mentions & files do we have to put in the repo and in the headers). So that we can compare our options here pray

@blenzi a few inputs if I may

* Yes it's possible, there are many examples on GNU website and you can check by yourself with github: when you create a repo, you can choose the license and whether you choose MIT our GPL, the copyright does appear :) Just picture it as an identifier of the authors/contributors

* I agree with both of you that we may add a workflow for this! But it would be a waste not to let the opportunity to someone else do the PR hehe (happy to help ok_hand )

* Licence is British English while License is for the US, but since we do already have a LICENSE file, I just edited the name of the PR to avoid confusion!

Thanks for fixing the PR title. Actually, I confused the US License with the French Licence (not easy to switch very often between the two when writing)
Following @blenzi review, I was already writing tests, your review on these is welcome

Akilditu
Akilditu previously approved these changes Jun 11, 2020
Copy link
Contributor

@Akilditu Akilditu 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 the Update, nice smooth checks :)

Wouldn't have done better 👍

@x0s
Copy link
Contributor Author

x0s commented Jun 11, 2020

I checked the others GPL licenses and I found the AGPL or also this clear explanation.
It's the same as GPLv3 but with one more restriction/protection for the freedom of the project. It's quite subtle but could be important.
As far as I understand, with the GPLv3, if someone get the project, use it for it's own service but without a form of (direct) distribution, like the Software as a Service (SaaS) model can do, so this person can keep his own improvements for himself.

So with the GPLv3, someone could be able to create a service that will evaluate an image with the Pyronear program and then return the evaluation. OK until here.
Now, if he decides to improve the program and to provide these improvements through his service, he will not have any obligation to publish these improvements.
Because it's a service, it is not considered as a distribution.

It's also a way to keep the big clouds of the world from turning one's project into their product. Small wonder, then, that some companies that license their code under the AGPL internally describe it as the "Amazon GPL." AWS, for example, has made orders of magnitude more money from MySQL than MySQL AB (and now Oracle) ever hoped to make (through RDS). By licensing with the AGPL, these companies ensure that no one besides themselves can monetize the project.

Source

As our position seems to be to keep this project as more as possible as a public property, I suggest to use the AGPLv3 instead of GPLv3.

I read a little bit on AGPLv3 also, that's interesting, I thought it may be used in the code that will serve the models, but yes it could also be applied here. I was just worried to be really careful about not disclosing confidential stuff (password, IPs, tokens...) due to mishandling when releasing the server code. Thank you for making us aware this possibility, I'll read more to get the subtleties of it.

[Update]
Also, it seems that CeCILL v2 already applies when the lib is distributed over the network

@x0s
Copy link
Contributor Author

x0s commented Jun 15, 2020

Hey, a little update !

If anybody is against moving to AGPLv3 (following @2ndcouteau suggestion), I will update the PR. If not, let's discuss, we need a consensus on this PR.

Additionally, I guess we can keep CeCILL V2.1 because it seems to be already robust to SaaS loophole while offering compatibility with French law:

J'ai modifié un logiciel sous CeCILL et les utilisateurs y accèdent via un site Web. Dois-je fournir les sources si l'un d'eux le demande ?

Oui, tout à fait. Si le 5.3.2 parle de distribution, il s'agit, comme indiqué en préambule de l'article 5.3, de "diffuser, de transmettre et de communiquer le logiciel au public, sur tout support et par tout moyen". Une mise à disposition via un site Web ou un serveur est donc bien une distribution même si l'utilisateur ne dispose pas d'un exemplaire du logiciel en propre, et l'obligation de fournir le code source du logiciel modifié s'applique.

What're your thoughts ?

@MateoLostanlen
Copy link
Member

Hi @x0s, thanks for this PR and for raising this point! I totally agree with @2ndcouteau, AGPLv3 seems to be the most suitable license. The coupling with CeCILL V2.1 is interesting considering the fact we mostly be focus on France in a first step. It's all good for me :)

MateoLostanlen
MateoLostanlen previously approved these changes Jun 22, 2020
@x0s x0s requested review from Akilditu, frgfm and 2ndcouteau June 23, 2020 07:48
Copy link

@2ndcouteau 2ndcouteau left a comment

Choose a reason for hiding this comment

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

If the point is valided, all the reference to GPL should be change to AGPL

LICENSE Outdated Show resolved Hide resolved
LICENSE Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@2ndcouteau
Copy link

I just picked some samples, but the fix from GPL to AGPL should be done on all the files. Thx

Copy link
Member

@frgfm frgfm 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, thanks a lot for all the investigation!

Copy link
Member

@fe51 fe51 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 as well.
AGPLv3 seems to best meet our needs, and CeCILL V2.1 fits our current scope.

Also, just to be sure, will we need to update copyright dates range while we update the repo (2019-2021 and so on) ?

@x0s x0s requested a review from 2ndcouteau June 24, 2020 07:20
@x0s
Copy link
Contributor Author

x0s commented Jun 24, 2020

Looks good to me as well.
AGPLv3 seems to best meet our needs, and CeCILL V2.1 fits our current scope.

Also, just to be sure, will we need to update copyright dates range while we update the repo (2019-2021 and so on) ?

Thanks for your review. Yes, with each passing year, the project survival has to be engraved in the git stone ;)

Copy link
Contributor

@Akilditu Akilditu left a comment

Choose a reason for hiding this comment

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

Everything's fine on my side !

@x0s x0s merged commit 0a47d8d into pyronear:master Jun 26, 2020
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.

7 participants