aboutcode-org / deltacode

DeltaCode: compare two codebase scans (from ScanCode) to detect significant changes.
http://www.aboutcode.org/
20 stars 27 forks source link

Simplify deltacode output #16

Closed steven-esser closed 6 years ago

steven-esser commented 6 years ago

For the csv output, it would be a better presentation if we simply included a single path value, instead of empty or repeated paths that are redundant.

steven-esser commented 6 years ago

@johnmhoran Plan of attack:

  1. Review + merge PR #20
  2. Modify Delta object to_dict function to return only minimal info (category and path for the most part)
  3. Modify CSV ouput function to handle new data structure
  4. Modify JSON output to Add header information that has been removed from DeltaCode.to_dict()
steven-esser commented 6 years ago

Feel free to push your changes in a branch without opening a PR. This is easier and less noisy than constantly updating a PR.

If you have questions or would like me to expand in detail on any of the above items, let me know.

johnmhoran commented 6 years ago

@majurg I've modified Delta.to-dict() and the CSV output to handle the new data structure, and fixed the 9 failing tests as well. About to add missing headers to the JSON output. Here's an excerpt from the current JSON output file testing a set of test scans with 1 added file:

{
    "added": [
        {
            "category": "added", 
            "path": "a/a5.py"
        }
    ], 
    "removed": [], 
    "modified": [], 
    "unmodified": [
        { . . .

How do we want to handle the redundant category information? Replace the 4 category keys with a single key named deltas and a value containing a list of category/path key/value pairs? Example:

{
    "deltas": [
        {
            "category": "added", 
            "path": "a/a5.py"
        },
        {
            "category": "unmodified", 
            "path": "a/a1.py"
        } . . .

Also, I assume we want the missing header info added to the top of the JSON output file. If so, I think that means adding values to the top of the incoming OrderedDict created by DeltaCode.to_dict(). Do I have that right??

johnmhoran commented 6 years ago

@majurg I just committed and pushed my work to date so you can vet including in connection with my recent questions.

steven-esser commented 6 years ago

We can keep the redundant categories around for now. If you recall our conversation yesterday, the delta object's category field will no longer match the Deltas dict category keys once we add license/copyright information.

johnmhoran commented 6 years ago

I do recall, e.g., license_change. Thanks @majurg . When you have a chance, can you sketch out an example excerpt of how you see the future JSON structure?

Re the 2nd question, do we want to add the missing header info to the top of the incoming OrderedDict. If yes, how does one do that? I've seen 2 approaches: rewrite the OrderedDict -- said to be slow but relatively straightforward -- or write a function to prepend.

Finally, once I finish this, what shall I tackle next?

johnmhoran commented 6 years ago

@majurg I can add the version header from inside the JSON function by moving the variable from __init__.py to cli.py, but adding the stats from DeltaCode.get_stats() has eluded me so far. Do we need to restore that to the DeltaCode.to_dict() method in order to add it to the JSON output file?

johnmhoran commented 6 years ago

@majurg I'm able to add the stats by passing new and old to generate_json and calling the stats like this: data['deltacode_stats'] = DeltaCode(new, old).get_stats(). The 2 headers (version and stats) have been added to the JSON file, though they appear at the bottom rather than the top (per my question above re adding to the top).

steven-esser commented 6 years ago
a = OrderedDict([
    ('header', header_info),
    ('deltacode_stats' deltacode.get_stats()),
    ('deltas', deltacode.to_dict())
])

pass the deltacode object into generate_json instead of the data dict and just call to_dict inside the csv function at the right place.

steven-esser commented 6 years ago

Shouldnt need to move version to cli.py. Just import deltacode at the top and reference the version like: deltacode.__version__

johnmhoran commented 6 years ago

Very nice. Thanks, @majurg .

My only open issue: the list comprehension you suggested -- still digging into how to include the 3 variable assignments and the tuple_list.append() operation currently inside the 2nd of the nested for loops.

johnmhoran commented 6 years ago

@majurg I believe I've answered my last question. Spent time yesterday trying to figure out how to address the double-nested multiple variable assignments and the append operation inside a list comprehension. Nothing seemed to work, no hints from my research.

Took a fresh look this morning and realized that I've seen this before in simpler form: initializing variables earlier than necessary. When all is said and done, it's just an append operation.

And this:

    tuple = ()
    tuple_list = []
    deltas = data

    for delta in deltas:
        category = delta
        for f in deltas[delta]:
            category = f['category']
            path = f['path']
            tuple = (category, path)
            tuple_list.append(tuple)

. . . can be replaced with this:

    deltas = data
    tuple_list = [(f['category'], f['path']) for delta in deltas for f in deltas[delta]]

All 79 tests pass. I want to do a little command-line testing just to be sure, and if all looks good, will clean up the code, commit, push and open a PR.

steven-esser commented 6 years ago

21 merged, closing this.