-
Notifications
You must be signed in to change notification settings - Fork 1
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Features/add logging #60
base: dev
Are you sure you want to change the base?
Conversation
f5e1a3b
to
fccb356
Compare
…/digipipe into features/add_logging
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this feature including all different ways of running tasks - works well so far! 😸
Some thoughts from my side:
- I like the idea of putting the dataset's folder where it belongs to. However, I find it a bit messy when it is saved to the data folder - the user wants to see the data quite often whereas the log is only checked if something goes wrong. What about a separate log folder?
- I'm a bit curious about the file name. You use the actual file's name (without extension), but what if there're multiple files?
What aboutlog/<DATASET_NAME>.log
(in folder I proposed above)? Or just sth likelog/dataset.log
which would save implementing an additional function to obtain the dataset's name. We'd haveDATASET_PATH / "log" / "dataset.log"
everywhere which is sufficient to my mind. Are you fine with that?- Sidenote: If we use wildcards and 10 jobs are working on 10 parallel tasks of the same dataset, everything will be logged to the same file, but I propose not to tackle this now.
- Currently there's no logging of Linux shell commands' output. That can be easily achieved by redirecting Linux' standard output (stdout) and standard error (stderr) messages by adding
2>&1 > {log}
(it's also described in the snakemake docs). I can do this.
One more thing: I'm getting all output twice at the moment (regular out from snakemake and additionally from the logger):
Could you suppress snakemake's log output? Or the output from our logger but it is richer than snakemake's so I'd prefer to see ours..
digipipe/store/datasets/bkg_vg250_districts_region/scripts/create.py
Outdated
Show resolved
Hide resolved
One more question: |
Thanks for your thoughts! I like the idea putting the log output to a separate directory! Since I wanted to keep the name of the data file in the content of the logging file, I decided to use the option of individual log files in the directory '.log' instead of the variant '.log/dataset.log'. The latter would require more adjustment and time to keep the name of the corresponding folder with datasets in it. Do you go along with that? |
That would be awesome! Thank you. :) |
I am quite at a loss as to how to silence this. Most of the entries have disappeared with commit a0a0676. Unfortunately I can't get rid of the ones you describe. I only found the |
Fixes #21.
With this PR logging is added to the pipeline.
Before merging into
dev
-branch, please make sure thatblack
andisort
.CHANGELOG.rst
was updated.