Skip to content

Conversation

@DaphneCD
Copy link
Contributor

This is a segmentation extension using Graph Cut and Star Shape algorithm.

@jcfr
Copy link
Member

jcfr commented Aug 27, 2015

Looking good 👍

Few more things that need clarification:


  1. Since the logic make use of MAXFLOW implementation restricting the use of the code and corresponding compiled code for research only.

See https://github.com/DaphneCD/GraphCutSegmentExtension/blob/58d8281eea9755faddb9eb2a4caed7a423c85988/GraphCutInteractiveSegmenter/Logic/TumorSegm/BK/README.TXT#L52-L53

Could you explicitly mention in the extension description that the extension can not be used for commercial application ?


  1. I also notice that there are header files without license/copyright info in the TumorSegm folder: AdaptiveSegment3D.h, VectOperation.h, ...

see https://github.com/DaphneCD/GraphCutSegmentExtension/tree/58d8281eea9755faddb9eb2a4caed7a423c85988/GraphCutInteractiveSegmenter/Logic/TumorSegm

Did you write those ?

Does this mean that they are associated with Apache License ?


  1. The license file you added doesn't reference any copyright owner. See https://github.com/DaphneCD/GraphCutSegmentExtension/blob/58d8281eea9755faddb9eb2a4caed7a423c85988/License.txt#L189

@pieper @fedorov @lassoan Do you have any comments ?

@pieper
Copy link
Member

pieper commented Aug 27, 2015

Thanks @DaphneCD for working on this extension! And thanks @jcfr for careful review!

DaphneCD please understand we need to be very careful with these issues related to code history and licensing. If you could, either here or on private email, please let us know your goals and the background of your project so we can help get your contributions integrated.

@jcfr It occurs to me that we should probably clarify the wiki with guidelines for preparing extension code. Looking at [1], I think we should say that while people can develop Category 3 extensions (closed source) they cannot be included in the Slicer Extension Catalog for legal and practical reasons and must be distributed through other means. We could also say Category 2 extensions need to be carefully reviewed because some "Open Source" licenses may cause problems if they can be interpreted to apply to the main Slicer distribution (I'm worried about extensions being seen as a workaround for putting GPL code in Slicer). Also we don't really address this question of "research only" extensions. Perhaps we should discuss this on a future hangout.

[1] http://www.slicer.org/slicerWiki/index.php/Documentation/Nightly/SlicerApplication/ExtensionsManager#What_is_an_extension_.3F

@fedorov
Copy link
Member

fedorov commented Aug 27, 2015

@jcfr since you asked, I think this is a great example that brings out limitations of the current process of integrating extensions and the application overall.

  1. I think many developers don't pay attention to the license, all of them without any evil intent I am sure: I did a quick check, and there are extensions that are integrated already but do not have a license file in the root of the repository. In this instance, I remembered to ask the contributor about the license just because I heard about the graph cut algorithm licensing issues some time in the past.

  2. it is not feasible to do a review of extension to affirm there is no licensed code included. In this particular situation, a whole library is copied together with the original license. But in some cases a function can be copied from another source. So the contributor should bear the responsibility if there is a breach, not 3D Slicer (if this is possible at all, from the legal perspective!).

To address 1) I suggest the contribution process is modified to automatically require license file to be present, and perhaps choose the type of license from the predefined types and specify it in the s4ext file (perhaps a license URL from http://opensource.org/licenses must be required?)

For 2) it should be made clear to the Slicer users that extensions installed may be governed by a license different from that used by the main application. There probably should be a disclaimer somewhere on first startup of the application and/or at the time extension is installed.

@lassoan
Copy link
Contributor

lassoan commented Aug 27, 2015

Limiting extensions to BSD-type licenses would have very bad consequences. I know several groups for who would be happy to share their work with the research community but not ready to decline all potential future commercial opportunities.

I think the only issue is that the user may install an extension that comes with a restrictive license without seeing the licensing terms. We could require all category 1 and 2 extensions to come with a specific license (Slicer license). For category 3 extensions we could show a popup when the user clicks on "Install" explaining that the licensing terms of this extension may be different from 3D Slicer core and that the user should review the licensing terms on the extension homepage and provide a link to the extension homepage, and Accept & Cancel buttons. This would not require any extra field in the s4ext file and implemented completely in the Slicer core (no MIDAS change needed).

Not requiring popup for category 1&2 extensions would be an incentive for developers to distribute their work as category 1&2, as these extensions appear more as part of Slicer and a bit simpler to install.

@pieper
Copy link
Member

pieper commented Aug 27, 2015

It will be good to fully clarify the meanings of the three categories in
this respect and to make sure users have the opportunity to easily
understand the implications of each of the categories.

We currently don't have a mechanism to display whether an extension is
category 1 or 2 in the extension manager (do we?). I think category 1
extensions (under the Slicer / BSD license) should install with no warnings
and category 2 should require a popup of the type Andras describes. (Of
course, Slicer and category 1 extensions shouldn't be blindly trusted, but
those are the ones we can have the most control over.)

And the fact that we won't accept category 3 code in the Extension Catalog
is not clearly stated in our current documentation [1,2]. People need to
set up alternate distribution methods for category 3 code. We might
consider a generic popup warning whenever installing an extension from an
unknown source (like modern OSes do).

http://www.slicer.org/slicerWiki/index.php/Documentation/Nightly/Extensions/CatalogPolicies#Requirements_for_Slicer_Extensions

http://www.slicer.org/slicerWiki/index.php/Documentation/Nightly/Developers/Tutorials/BuildTestPackageDistributeExtensions

On Thu, Aug 27, 2015 at 9:50 AM, Andras Lasso notifications@github.com
wrote:

Limiting extensions to BSD-type licenses would have very bad consequences.
I know several groups for who would be happy to share their work with the
research community but not ready to decline all potential future commercial
opportunities.

I think the only issue is that the user may install an extension that
comes with a restrictive license without seeing the licensing terms. We
could require all category 1 and 2 extensions to come with a specific
license (Slicer license). For category 3 extensions we could show a popup
when the user clicks on "Install" explaining that the licensing terms of
this extension may be different from 3D Slicer core and that the user
should review the licensing terms on the extension homepage and provide a
link to the extension homepage, and Accept & Cancel buttons. This would not
require any extra field in the s4ext file and implemented completely in the
Slicer core (no MIDAS change needed).

Not requiring popup for category 1&2 extensions would be an incentive for
developers to distribute their work as category 1&2, as these extensions
appear more as part of Slicer and a bit simpler to install.


Reply to this email directly or view it on GitHub
#1065 (comment)
.

@DaphneCD
Copy link
Contributor Author

Thank you for all the comments.

My project used the code of a former RA which integrated Graph Cut and Star Shape algorithm. For the convenience of some medical researchers, I need to transform the original interface to Slicer.

Actually, the project is still under improvement. We need the advice and comments from medical researchers for further enhancement. To avoiding their building Slicer from source, I think contributing the code as an extension is a good choice.

Sorry that I neglect the license problem. I understand all your concerns. I think for now the code is only for research use. I'll talk with my supervisor and then clear all the problems.

@pieper
Copy link
Member

pieper commented Aug 27, 2015

That sounds great DaphneCD - I think this can be a category 2 extension
with the appropriate disclaimer about the non-commercial use restriction.

On Thu, Aug 27, 2015 at 10:14 AM, DaphneCD notifications@github.com wrote:

Thank you for all the comments.

My project used the code of a former RA which integrated Graph Cut and
Star Shape algorithm. For the convenience of some medical researchers, I
need to transform the original interface to Slicer.

Actually, the project is still under improvement. We need the advice and
comments from medical researchers for further enhancement. To avoiding
their building Slicer from source, I think contributing the code as an
extension is a good choice.

Sorry that I neglect the license problem. I understand all your concerns.
I think for now the code is only for research use. I'll talk with my
supervisor and then clear all the problems.


Reply to this email directly or view it on GitHub
#1065 (comment)
.

@DaphneCD
Copy link
Contributor Author

Hi,

I've confirmed with my supervisor. This project is only for research use. Should I still use the Apache license? And for the head files without copyright, they are the work of the previous RA and me. Can I simply just add copyright information at the beginning of each file?

Sorry that I'm not familiar with the license regulation.

Thanks for the reviews and instructions.

@DaphneCD
Copy link
Contributor Author

DaphneCD commented Sep 1, 2015

Hi,

I've added the copyright in the front of the header files under TumorSegm folder and cleared that the code is only for research use only.

Please let me know if there is any more problems.

Thanks for your time.

@DaphneCD
Copy link
Contributor Author

Hi,

I'm still waiting my code to be reviewed.
Any feedback will be appreciated.

Thanks.

@jcfr
Copy link
Member

jcfr commented Sep 14, 2015

@DaphneCD Thanks for the reminder.

One last thing, could you update the description adding a sentence similar to this one: This extension can be used for research purposes ONLY. It can NOT be used for commercial purposes.

@pieper @fedorov @lassoan Any thing to add ?

@pieper
Copy link
Member

pieper commented Sep 14, 2015

That make sense to me - thanks again for the contribution @DaphneCD!

On Mon, Sep 14, 2015 at 10:24 AM, Jean-Christophe Fillion-Robin <
notifications@github.com> wrote:

@DaphneCD https://github.com/DaphneCD Thanks for the reminder.

One last thing, could you update the description adding a sentence similar
to this one: "This extension can be used for research purposes ONLY. It can
NOT be used for commercial purposes."

@pieper https://github.com/pieper @fedorov https://github.com/fedorov
@lassoan https://github.com/lassoan Any thing to add ?


Reply to this email directly or view it on GitHub
#1065 (comment)
.

@DaphneCD
Copy link
Contributor Author

I've updated the description file and opened a new pull request.

@jcfr
Copy link
Member

jcfr commented Sep 14, 2015

Closing. Superseded by #1093

@jcfr jcfr closed this Sep 14, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

5 participants