Only pass score during Delta creation.

[steven-esser]

change to Delta(score, new_file, old_file) in all the places that it was created.

This involves removing 'category' from the Delta object

Our initial scores should be simple for now: an Added should +100 to a score (which is 0 by default). A Removed will simply not add any value to a score. Both cases will also have the factors: 'added' or 'removed' appending to the Delta.factors

During license_diff and other DeltaCode steps, we will simply add values to the score as things are found out about the Delta.

[johnmhoran]

Update: As noted below, this score-based approach seems to be working:

added = len([i for i in self.deltas if i.score == 100])

@MaJuRG I have a question prompted by our discussions yesterday and today about our transition from a category-based to a score-based structure:

• What should we do with DeltaCode.get_stats(), which currently returns an OrderedDict whose keys are the former categories added, modified, moved, removed and unmodified?

My current approach (where i is a Delta object) is:

        added = len([i for i in self.deltas if 'added' in i.factors])
        modified = len([i for i in self.deltas if 'modified' in i.factors])
        moved = len([i for i in self.deltas if 'moved' in i.factors])
        removed = len([i for i in self.deltas if 'removed' in i.factors])
        unmodified = len([i for i in self.deltas if 'unmodified' in i.factors])

get_stats() currently omits the three license-specific categories we formerly added through Delta._license_diff() (now handled in the DeltaCode object).

Preserving the category information strikes me as useful to the user, but you may have a different view. A few thoughts:

• If we want to retain that info (as we do in the Delta.factors list) and continue providing stats, I think we ought to add the three license-specific categories.
• However, the only method I'm aware we could use would require an "if [category] in i.factors" condition, which as you pointed out is undesirable given its use of/reliance on maintaining and retrieving strings.
• Since we're appending categories to the Delta.factors list, each of the three license-specific categories will be in a Delta.factors list that also contains modified, requiring a condition something like

if 'modified' in i.factors and 'license' not in i.factors

This is not urgent -- we can discuss on Monday, and in the interim I'll take the "... in i.factors" approach I've described.

[johnmhoran]

@MaJuRG FYI, I've decided to change the score for moved from 0 to 5 in order to distinguish moved from unmodified (whose score is also 0). This will enable us to replace a category-based condition with a score-based condition in utils.deltas(), i.e., elif delta.category != 'unmodified' => elif delta.score != 0.

[johnmhoran]

@MaJuRG One other note: re my comment above regarding get_stats(), this score-based approach seems to be working well:

added = len([i for i in self.deltas if i.score == 100])

[steven-esser]

Well we probably either need to remove the method or change it to reflect # of deltas at or above/below certain score thresholds.

I say we just remove it that way we don’t need to maintain it. Not sure that anyone is actually using it in the output (Michael is not) so we should be ok.

We could also just move the current get_stats code to a utility test function so we don’t have to change a bunch of tests (if get_stats is even used there)

[johnmhoran]

We call get_stats() directly in 23 tests in test_deltacode.py, and indirectly in each test that produces a JSON output file through the write_json() method (I haven't done a count of those tests).

Since the refactored get_stats() seems to be working OK, perhaps we should keep as is unless/until our future work on dynamic scoring requires a change.

Alternatively, we might want to open a separate issue for get_stats() so that it can be addressed independent of the existing refactoring issues. (I'm currently working on #62, #63 and #64 together since they seem dependent on one another. I think I have most if not all of the underlying code refactoring done, now working on the numerous tests affected by the changes from Delta.category to Delta.factors and Delta._license_diff() to DeltaCode.license_diff().)

These are the current refactored variable definitions in get_stats():

        added = len([i for i in self.deltas if i.score == 100])
        modified = len([i for i in self.deltas if i.score == 20])
        license_change = len([i for i in self.deltas if i.score == 30])
        license_info_removed = len([i for i in self.deltas if i.score == 35])
        license_info_added = len([i for i in self.deltas if i.score == 40])
        moved = len([i for i in self.deltas if i.score == 5])
        removed = len([i for i in self.deltas if i.score == 10])
        unmodified = len([i for i in self.deltas if i.score == 0])

[steven-esser]

@johnmhoran We can keep it for now if you want; its up to you really. The reason why I dont want to keep it is because we have to constantly change/tweak it since its involved in so many tests, which is annoying for something that no many people use.

I would say for now and in the future when writing tests: lets avoid using get_stats().

[johnmhoran]

@MaJuRG Understood. Since we don't need get_stats() going forward, I'll remove it as part of my current refactoring.