🚀 Add information to app summary output for what will be run on --commit (#27)

[Keimya]

app.py
In the first invocation without running commit:

• "RAN" is changed to "STAGED"
• "SKIPPED" analyses are separated into SUCCEEDED, FAILED, also indicating if results are protected

In the second invocation upon --commit:

• "RAN" is displayed as the sum of run analyses + unprotected succeeded analyses

scenarios run for protected and unprotected results, and when commit = True and commit = False

test_app.py

• def test_commit_description added

• def test_get_experiments_from_default_cli_options modified

• 🐛   Bug fix

• 🚀   New feature

• 🔧   Code refactor

• ⚠️   Breaking change that would cause existing functionality to change

• 📘   I have updated the documentation accordingly

• ✅   I have added tests to cover my changes

[codecov]

Codecov Report

Patch coverage: 96.00% and project coverage change: -0.01 ⚠️

Comparison is base (cef1f66) 95.24% compared to head (4b0ff05) 95.24%.

Additional details and impacted files

@@            Coverage Diff             @@
##           master      #53      +/-   ##
==========================================
- Coverage   95.24%   95.24%   -0.01%     
==========================================
  Files          19       19              
  Lines        2336     2357      +21     
==========================================
+ Hits         2225     2245      +20     
- Misses        111      112       +1     

Impacted Files	Coverage Δ
isabl_cli/app.py	98.69% <96.00%> (-0.11%)	⬇️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

[juanesarango]

Thanks for the PR. It's getting a good shape.
Following @funnell's suggestions from #27 what if we do it like this:

(1) When analyses are protected:

• First invocation without --commit:

STAGED 150 | SKIPPED 100 | INVALID 0

Add --commit to run 150 analyses:

• Second invocation with --commit:

RAN 150 | SKIPPED 100 | INVALID 0

(2) When analyses are NOT protected:

• First invocation without --commit:

STAGED 150 | SKIPPED 100 | INVALID 0

SKIPPED 100:
    97   SUCCEEDED (Unprotected)
    3     FAILED analyses will be skipped

Add --commit to run 247 analyses:

• Second invocation with --commit:

RAN 247 | SKIPPED 3 | INVALID 0

This way in the default scenario where analyses are protected we don't need to detailed each one of the skipped ones saying (Protected).

What do you think? @funnell @Keimya

[Keimya]

[Keimya]

updated output:
protected results

unprotected results

[funnell]

I think I still prefer the message saying something like this for unprotected:

Add --commit to run 247 analyses:
150  STAGED
97   SUCCEEDED (Unprotected)

You could remove the listing of what was skipped if that's too verbose.

[funnell]

to be more explicit, what I'm suggesting (incorporating your feedback @juanesarango) would be:

1. Results Unprotected

• without --commit

STAGED 1 | SKIPPED 2 | INVALID 0

Add --commit to run 2 analyses:
1  STAGED
1  SUCCEEDED (Unprotected)

• with --commit

RAN 2 | SKIPPED 1 | INVALID 0

2. Results Protected

• without --commit

STAGED 1 | SKIPPED 2 | INVALID 0

Add --commit to run 1 analysis

• with --commit

RAN 1 | SKIPPED 2 | INVALID 0

Although, I think it could actually be nice to have a summary after running with --commit as well

[funnell]

Another comment I have, that is sort of related, is that I would actually prefer it if Isabl didn't insert analysis rows into the database until --commit. The problem I've encountered is misspecifying the filters and all of a sudden there's a bunch of unwanted analysis records.

What do you think of something like:

1. no --commit, just check for validity and report how many new analyses would be created, how many unprotected would be run, how many are invalid
2. with --commit, create new analysis records, and run them?

edit: #55

[funnell]

@juanesarango I just noticed that the cli already prints this at the end of a non-commit run:

Add --commit to proceed.

so we would end up with something like this?

STAGED 1 | SKIPPED 2 | INVALID 0

Add --commit to run 2 analyses:
1  STAGED
1  SUCCEEDED (Unprotected)

Add --commit to proceed.

Perhaps the new wording should be changed then? maybe Add --commit to run 2 analyses: should be changed to something like Ready to run 2 analyses: or 2 analyses available to run:

[juanesarango]

Thanks @Keimya. I like @funnell suggestions. The whole message can be then something like:

STAGED 150 | SKIPPED 100 | INVALID 0

247 analyses available to run:
    150  STAGED
    97   SUCCEEDED (Unprotected)

Add --commit to proceed

[Keimya]

6/16/2023
Protected

Unprotected

[juanesarango]

Great work @Keimya! Thanks a lot for the contribution! 🙌🏽