-
-
Notifications
You must be signed in to change notification settings - Fork 27
Add license category-change detection and scoring #86
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
Changes from 2 commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -58,8 +58,10 @@ def __init__(self, new_path, old_path, options): | |
| self.determine_moved() | ||
| self.license_diff() | ||
| self.copyright_diff() | ||
| # Sort deltas by score, descending, i.e., high > low. | ||
| # Sort deltas by score, descending, i.e., high > low, and then by | ||
| # factors, alphabetically. | ||
| self.deltas.sort(key=lambda Delta: Delta.score, reverse=True) | ||
| self.deltas.sort(key=lambda Delta: Delta.factors, reverse=False) | ||
|
|
||
| def align_scans(self): | ||
| """ | ||
|
|
@@ -134,7 +136,7 @@ def determine_delta(self): | |
| # This file already classified so do nothing | ||
| new_index[path] | ||
| except KeyError: | ||
| delta = Delta(10, None, old_file) | ||
| delta = Delta(0, None, old_file) | ||
| delta.factors.append('removed') | ||
| self.deltas.append(delta) | ||
| continue | ||
|
|
@@ -158,10 +160,10 @@ def determine_moved(self): | |
| both indices with the same 'sha1' and File 'name' attributes, and | ||
| converting each such pair of 'added' and 'removed' Delta objects to a | ||
| 'moved' Delta object. The 'added' and 'removed' indices are defined by | ||
| the 'score' attribute of the Delta objects. | ||
| the presence/absence of the object's 'old_file' and 'new_file'. | ||
| """ | ||
| added = self.index_deltas('sha1', [i for i in self.deltas if i.score == 100]) | ||
| removed = self.index_deltas('sha1', [i for i in self.deltas if i.score == 10]) | ||
| added = self.index_deltas('sha1', [i for i in self.deltas if i.old_file is None and i.new_file]) | ||
| removed = self.index_deltas('sha1', [i for i in self.deltas if i.old_file and i.new_file is None]) | ||
|
|
||
| # TODO: should it be iteritems() or items() | ||
| for added_sha1, added_deltas in added.iteritems(): | ||
|
|
@@ -177,7 +179,7 @@ def update_deltas(self, added, removed): | |
| 'moved' Delta object -- passing the appropriate 'score' during object | ||
| creation -- and delete the 'added' and 'removed' objects. | ||
| """ | ||
| delta = Delta(5, added.new_file, removed.old_file) | ||
| delta = Delta(0, added.new_file, removed.old_file) | ||
| delta.factors.append('moved') | ||
| self.deltas.append(delta) | ||
| self.deltas.remove(added) | ||
|
|
@@ -187,22 +189,41 @@ def license_diff(self): | |
| """ | ||
| Compare the license details for a pair of 'new' and 'old' File objects | ||
| in a Delta object and change the Delta object's 'score' attribute -- | ||
| and add an appropriate category (e.g., 'license info removed', 'license | ||
| info added' or 'license change') to the Delta object's 'factors' | ||
| attribute -- if there has been a license change and depending on the | ||
| nature of that change. | ||
| and add one or more appropriate categories (e.g., 'license change', | ||
| 'copyleft added') to the Delta object's 'factors' attribute -- if there | ||
| has been a license change and depending on the nature of that change. | ||
| """ | ||
| for delta in self.deltas: | ||
| if 20 <= delta.score < 100: | ||
| unique_categories = set([ | ||
| 'Commercial', | ||
| 'Copyleft', | ||
| 'Copyleft Limited', | ||
| 'Free Restricted', | ||
| 'Patent License', | ||
| 'Proprietary Free' | ||
| ]) | ||
|
|
||
| unique_commercial_categories = set([ | ||
| 'Commercial', | ||
| 'Proprietary Free' | ||
| ]) | ||
|
|
||
| for delta in self.deltas: | ||
| if delta.is_modified(): | ||
| new_licenses = delta.new_file.licenses or [] | ||
| old_licenses = delta.old_file.licenses or [] | ||
|
|
||
| if len(delta.new_file.licenses) > 0 and delta.old_file.licenses == []: | ||
| new_categories = set(license.category for license in new_licenses) | ||
| old_categories = set(license.category for license in old_licenses) | ||
|
|
||
| if delta.new_file.has_licenses() and not delta.old_file.has_licenses(): | ||
| delta.update(20, 'license info added') | ||
| # no license ==> 'Copyleft Limited'or higher | ||
| for item in sorted(unique_categories): | ||
| if item in new_categories: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The logic should be reversed for line 221-222: |
||
| delta.update(20, item.lower() + ' added') | ||
| return | ||
|
|
||
| if delta.new_file.licenses == [] and len(delta.old_file.licenses) > 0: | ||
| if not delta.new_file.has_licenses() and delta.old_file.has_licenses(): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. We should put this block (226-228) near the top, as it is the least logically complex code block. |
||
| delta.update(15, 'license info removed') | ||
| return | ||
|
|
||
|
|
@@ -211,6 +232,13 @@ def license_diff(self): | |
|
|
||
| if new_keys != old_keys: | ||
| delta.update(10, 'license change') | ||
| for item in sorted(new_categories - old_categories): | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Not sure we need to sort here
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @MaJuRG I sorted here (and earlier, line 221, mentioned in your comment above) to display the license category-change factors alphabetically -- i.e., in a consistent order -- thinking that would make it easier for a user to compare and analyze factors. Removing the sort in both locations works fine -- I just need to modify a few failing tests in which I
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets remove it from both; if anything this will incur a performance cost.
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Using
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Does
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes it works -- I was referring to the modifications needed to fix the failing related tests -- there, we can use either
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, in the test cases it is up to you; sorting test results is always fine. |
||
| # 'Permissive' or 'Public Domain' ==> 'Copyleft Limited' or higher | ||
| if len(old_categories & unique_categories) == 0 and item in unique_categories: | ||
| delta.update(20, item.lower() + ' added') | ||
| # anything ==> 'Proprietary Free' or 'Commercial' | ||
| elif item in unique_commercial_categories: | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lets not have a separate distinction/process for commercial licenses. meaning forget about my mentioning "anything ==> Commercial" in tickets or otherwise. |
||
| delta.update(20, item.lower() + ' added') | ||
|
|
||
| def copyright_diff(self): | ||
| """ | ||
|
|
@@ -222,15 +250,14 @@ def copyright_diff(self): | |
| nature of that change. | ||
| """ | ||
| for delta in self.deltas: | ||
| if 20 <= delta.score < 100: | ||
|
|
||
| if delta.is_modified(): | ||
| new_copyrights = delta.new_file.copyrights or [] | ||
| old_copyrights = delta.old_file.copyrights or [] | ||
|
|
||
| if len(delta.new_file.copyrights) > 0 and delta.old_file.copyrights == []: | ||
| if delta.new_file.has_copyrights() and not delta.old_file.has_copyrights(): | ||
| delta.update(10, 'copyright info added') | ||
| return | ||
| elif delta.new_file.copyrights == [] and len(delta.old_file.copyrights) > 0: | ||
| if not delta.new_file.has_copyrights() and delta.old_file.has_copyrights(): | ||
| delta.update(10, 'copyright info removed') | ||
| return | ||
|
|
||
|
|
@@ -289,6 +316,26 @@ def update(self, score=0, factor=''): | |
| self.factors.append(factor) | ||
| self.score += score | ||
|
|
||
| def is_modified(self): | ||
| """ | ||
| Identify a Delta object meriting attention to possible changes in its | ||
| license or copyright content because the File object has been modified. | ||
| """ | ||
| if self.score > 0 and self.old_file: | ||
| return True | ||
|
|
||
| def is_unmodified(self): | ||
| """ | ||
| Since 'unmodified' is no longer the only category/factor with a | ||
| score = 0, test the Delta object's attributes for categories/factors | ||
| other than 'unmodified' and return True if all but 'unmodified' are | ||
| ruled out. | ||
| """ | ||
| if (self.old_file and self.new_file and | ||
| self.old_file.sha1 == self.new_file.sha1 and | ||
| self.old_file.path == self.new_file.path): | ||
| return True | ||
|
|
||
| def to_dict(self): | ||
| """ | ||
| Return an OrderedDict comprising the 'factors', 'score' and new and old | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| moved,5,b/a4.py,a4.py,file,200,a/a4.py | ||
| moved,0,b/a4.py,a4.py,file,200,a/a4.py |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| added,100,b/a4.py,a4.py,file,200, | ||
| added,100,b/a4_copy.py,a4_copy.py,file,200, | ||
| removed,10,a/a4.py,a4.py,file,200, | ||
| removed,0,a/a4.py,a4.py,file,200, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| added,100,b/a4.py,a4.py,file,200, | ||
| added,100,c/a4.py,a4.py,file,200, | ||
| removed,10,a/a4.py,a4.py,file,200, | ||
| removed,0,a/a4.py,a4.py,file,200, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| modified license info added copyright info added,50,path.txt,path.txt,file,300, | ||
| modified license info added copyleft added copyright info added,70,path.txt,path.txt,file,300, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| modified license info added copyright info removed,50,path.txt,path.txt,file,300, | ||
| modified license info added copyleft added copyright info removed,70,path.txt,path.txt,file,300, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| modified,20,some/path/c/c1.py,c1.py,file,300, | ||
| modified license change,30,some/path/a/a1.py,a1.py,file,300, | ||
| modified license change copyleft added,50,some/path/a/a1.py,a1.py,file,300, | ||
| modified license change,30,some/path/b/b1.py,b1.py,file,300, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,2 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| removed,10,a/a4.py,a4.py,file,200, | ||
| removed,0,a/a4.py,a4.py,file,200, |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,3 @@ | ||
| Factors,Score,Path,Name,Type,Size,Old Path | ||
| added,100,a/a4_renamed_not_modified.py,a4_renamed_not_modified.py,file,200, | ||
| removed,10,a/a4.py,a4.py,file,200, | ||
| removed,0,a/a4.py,a4.py,file,200, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lets not have a separate distinction/process for commercial licenses.
meaning forget about my mentioning "anything ==> Commercial" in tickets or otherwise.