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

Incorporate license-required notification text #3

Open
tsalo opened this issue Oct 26, 2020 · 6 comments
Open

Incorporate license-required notification text #3

tsalo opened this issue Oct 26, 2020 · 6 comments

Comments

@tsalo
Copy link
Member

tsalo commented Oct 26, 2020

I am not all that familiar with the requirements of the Apache 2.0 license (the one used by the main ICA-AROMA repo), but we can modify and redistribute the code, with some caveats. We need to incorporate information about the changes we have made into the source code and retain the original license. I'm a little fuzzy on the specific details, but we should review the license to figure out what exactly we need to do.

@eurunuela
Copy link
Collaborator

@smoia might know something about the Apache 2.0 license. I remember he went through different ones when choosing a license for phys2bids.

@smoia
Copy link

smoia commented Oct 27, 2020

Apache 2.0 is a very permissive license, slightly less than MIT license. It's more private/industry sector friendly than the GNU GPL or the Mozilla, and a tad more verbose than the very permissive ones.
You can find a recap of some license often adopted in GitHub here

@tsalo
Copy link
Member Author

tsalo commented Oct 27, 2020

Based on GitHub's summary, I think we just need the following:

  • A copy of the original license.
  • A copy of the original copyright notice.
  • Some documentation of the changes we've made to the original code.

I'm still a little uncertain about how that translates to actual files in the repo, but I think we can include the original license in a subfolder, with a new copy of the license in the regular LICENSE location (with the copyright changed). There is no separate copyright information, so I don't think we need to worry about that. Finally, we should probably use a changelog to explicitly track changes we've made since cloning the original code.

(b) You must cause any modified files to carry prominent notices stating that You changed the files; and

We also probably need to add a little notice that we have changed the contents of each file that is modified from the original code to the file's docstring. Maybe something like:

"""
This module does X.

This file has been modified from the original ICA-AROMA 
source code by the BrainHack Donostia team.
"""

What do you all think?

@oesteban
Copy link

oesteban commented Nov 10, 2020

Yes, this is correct - you need to indicate the changes.

This is why I would suggest minimizing the amount of code that you are actually bringing from ICA-AROMA.

EDIT: see https://github.com/Brainhack-Donostia/ica-aroma-org/issues/47#issuecomment-724506946

Instead, I would use the paper to base the new implementation. Follow through all the steps, look up into ICA-AROMA's current code and evaluate if that step can be transferred over. If so, state that the module has been derived from ICA-AROMA and keep track of those changes. If you ALWAYS make an initial commit with the untouched ICA-AROMA's code and a fixated commit message (e.g. maint(license): adding original ICA-AROMA's code), then you can easily make a diff with your HEAD and document the changes as required by the license.

@eurunuela eurunuela transferred this issue from Brainhack-Donostia/ica-aroma-org Nov 10, 2020
@tsalo
Copy link
Member Author

tsalo commented Nov 11, 2020

@oesteban thanks for outlining the changes we need to make and steps we can take.

One thing I want to note is that I think the features and classifier are both very fragile. I am not sure that any changes that make sense conceptually, but don't produce the exact same results, will produce "valid" results. I could be wrong, but I got this impression the last time I tried refactoring AROMA.

@oesteban
Copy link

One thing I want to note is that I think the features and classifier are both very fragile.

Yes, I would agree with this.

I am not sure that any changes that make sense conceptually, but don't produce the exact same results, will produce "valid" results. I could be wrong, but I got this impression the last time I tried refactoring AROMA.

You mean, for instance, by working on native space?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Todo
Development

No branches or pull requests

4 participants