Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
83 changes: 65 additions & 18 deletions src/deltacode/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
Expand Down Expand Up @@ -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
Expand All @@ -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():
Expand All @@ -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)
Expand All @@ -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([

Copy link
Copy Markdown
Contributor

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.

'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:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

The logic should be reversed for line 221-222:

for category in new_categories:
    if category in unique_categories:

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():

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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

Expand All @@ -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):

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Not sure we need to sort here

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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 assert [list A] == [list B], which I can replace with assert set([list A]) == set([list B]).

@steven-esser steven-esser Mar 16, 2018

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Using set() works here since there are no duplicates, but perhaps it's better to use sorted() instead. Tested, works.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Does for item in new_categories - old_categories: not work?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The 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 set() or sorted() to compare the factors list comprehension with the expected Delta.factors list and not worry about the order of the lists' elements.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The 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:

Copy link
Copy Markdown
Contributor

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.

delta.update(20, item.lower() + ' added')

def copyright_diff(self):
"""
Expand All @@ -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

Expand Down Expand Up @@ -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
Expand Down
10 changes: 9 additions & 1 deletion src/deltacode/models.py
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ def index_files(self, index_key='path'):
for f in self.files:
key = getattr(f, index_key)

if index.get(key) == None:
if index.get(key) is None:
index[key] = []
index[key].append(f)
else:
Expand Down Expand Up @@ -169,12 +169,20 @@ def get_licenses(self, dictionary):
else:
return [License(l) for l in dictionary.get('licenses')]

def has_licenses(self):
if len(self.licenses) > 0:
return True

def get_copyrights(self, dictionary):
if dictionary.get('copyrights') == []:
return []
else:
return [Copyright(l) for l in dictionary.get('copyrights')]

def has_copyrights(self):
if len(self.copyrights) > 0:
return True

def to_dict(self):
d = OrderedDict([
('path', self.path),
Expand Down
6 changes: 3 additions & 3 deletions src/deltacode/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,13 +45,13 @@ def collect_errors(deltacode):
def deltas(deltacode, all_delta_types=False):
"""
Return a generator of Delta dictionaries for JSON serialized ouput. Omit
all unmodified Delta objects -- identified by a 'score' of 0 -- unless the
user selects the '-a'/'--all' option.
all unmodified Delta objects unless the user selects the '-a'/'--all'
option.
"""
for delta in deltacode.deltas:
if all_delta_types is True:
yield delta.to_dict()
elif delta.score != 0:
elif not delta.is_unmodified():
yield delta.to_dict()


Expand Down
2 changes: 1 addition & 1 deletion tests/data/cli/1_file_moved.csv
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
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
unmodified,0,a/a3.py,a3.py,file,200,
unmodified,0,b/b4.py,b4.py,file,200,
unmodified,0,a/a2.py,a2.py,file,200,
Expand Down
2 changes: 1 addition & 1 deletion tests/data/cli/1_file_moved_all_not_selected.csv
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
2 changes: 1 addition & 1 deletion tests/data/cli/1_file_moved_and_1_copy.csv
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,
2 changes: 1 addition & 1 deletion tests/data/cli/1_file_moved_and_added.csv
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,
2 changes: 1 addition & 1 deletion tests/data/cli/copyright_and_license_info_added.csv
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,
2 changes: 1 addition & 1 deletion tests/data/cli/modified_new_license_added.csv
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,
2 changes: 1 addition & 1 deletion tests/data/cli/removed1.csv
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,
2 changes: 1 addition & 1 deletion tests/data/cli/renamed1.csv
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,
Loading