nils-braun / b2luigi

Task scheduling and batch running for basf2 jobs made simple
GNU General Public License v3.0
17 stars 11 forks source link

tasks can return all input or all outputfiles with a single call #111

Closed anselm-baur closed 3 years ago

anselm-baur commented 3 years ago

Hey guys, I added some functionalities to get with a single call all input or all output files of a task. For me this is a very nice debugging feature, using this together with the dry_run method.

class TheSuperFancyTask(b2luigi.Task):
    def dry_run(self):
        for name in self.get_all_input_file_names(): 
            print(f"\t\tinput:\t{name}") 
        for name in self.get_all_output_file_names(): 
            print(f"\t\toutput:\t{name}")
[....]

Producing s.th. like

MainTask                                                                                                                                                    [750/1824]
        Would run TheSuperFancyTask()
        call: dry_run()
                input:  /path/to/file/a
                input:  /path/to/file/b
                input:  /path/to/file/xyz
                output: /path/to/output/1
                output: /path/to/output/n 

when running in dry-run mode.

I know having several output files is not best practice, but if you have it and want to debug it fast, you have here some nice feature as well.

Are you interested?

codecov-commenter commented 3 years ago

Codecov Report

Merging #111 (48dc2bb) into main (8606fab) will increase coverage by 0.38%. The diff coverage is 93.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
+ Coverage   55.81%   56.19%   +0.38%     
==========================================
  Files          23       23              
  Lines        1453     1468      +15     
==========================================
+ Hits          811      825      +14     
- Misses        642      643       +1     
Impacted Files Coverage Δ
b2luigi/core/task.py 76.11% <93.33%> (+4.96%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8606fab...48dc2bb. Read the comment docs.

anselm-baur commented 3 years ago

Codecov Report

Merging #111 (81b5eb6) into main (8606fab) will decrease coverage by 0.29%. The diff coverage is 26.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #111      +/-   ##
==========================================
- Coverage   55.81%   55.51%   -0.30%     
==========================================
  Files          23       23              
  Lines        1453     1468      +15     
==========================================
+ Hits          811      815       +4     
- Misses        642      653      +11     

Impacted Files Coverage Δ
b2luigi/core/task.py 61.19% <26.66%> (-9.96%)
Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 8606fab...81b5eb6. Read the comment docs.

Well, very nice this feature... but what does it tell me?? It seems I made s.th. worse but I have no clue what...

nils-braun commented 3 years ago

Codecov shows you the code coverage during tests. In a perfect world, you would like to cover every like of code at least once in a unit test (otherwise you can never be sure if the line does what you think it should - in python it is even worse as lines are only evaluated if they are used - so you could even have things like undefined variables). The message tells you that you introduced new lines of code without testing them - to get rid of the message, just write some tests (what you should do anyways). Having this ad part of an automated test in PRs is good practice.

anselm-baur commented 3 years ago

Codecov shows you the code coverage during tests. In a perfect world, you would like to cover every like of code at least once in a unit test (otherwise you can never be sure if the line does what you think it should - in python it is even worse as lines are only evaluated if they are used - so you could even have things like undefined variables). The message tells you that you introduced new lines of code without testing them - to get rid of the message, just write some tests (what you should do anyways). Having this ad part of an automated test in PRs is good practice.

so you want me to write unit tests... I look what I can do :D

nils-braun commented 3 years ago

Sure, I always want that :-)

anselm-baur commented 3 years ago

it's green... GREEEN!!! :)

meliache commented 3 years ago

@anselm-baur @nils-braun What's the status, any help needed on this PR? There's still one unmerged suggestion and there's a couple of conversations (e.g. 1, 2), but I'm not sure if they have already been resolved. If so, maybe click the resolve conversation button available for you. Or is it just that you (@anselm-baur) are busy and didn't have time to work on resolving the comments, then maybe I could take a look and see if I can help.

anselm-baur commented 3 years ago

arigato gozaimashita!