print to logger
#448
print to logger
#448
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #448 +/- ##
==========================================
- Coverage 62.77% 62.70% -0.07%
==========================================
Files 41 41
Lines 6807 6824 +17
==========================================
+ Hits 4273 4279 +6
- Misses 2534 2545 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
This is something I've been wanting to do for ages but never gotten around to it.
A couple of q's:
- How does this look in a notebook? Does logging.info print to the notebook output (I think yes and but we should double check, I can do that if you like)
- What is the rationale behind replacing which print statements you have replaced?Will add some comments in the code to explain what I mean by this
mapreader/annotate/utils.py
Outdated
| print(f"[INFO] {new_labels} labels were not already stored") | ||
| print(f"[INFO] Total number of saved annotations: {len(image_df)}") | ||
| else: | ||
| print("[INFO] No annotations to save!") | ||
| logger.info("No annotations to save!") |
There was a problem hiding this comment.
e.g. here, should we replace all prints?
mapreader/classify/classifier.py
Outdated
| @@ -733,12 +737,12 @@ def train( | |||
| print_info_batch_freq=print_info_batch_freq, | |||
| ) | |||
| except KeyboardInterrupt: | |||
| print("[INFO] Exiting...") | |||
| logger.info("Exiting...") | |||
| if os.path.isfile(self.tmp_save_filename): | |||
| print(f'[INFO] Loading "{self.tmp_save_filename}" as model.') | |||
|
There is also a cprint function in the classifier that prints things in different colours: https://github.com/Living-with-machines/MapReader/blob/cc5a1ca4f6a16f98d09c31727bebb2fd2a9ed49b/mapreader/classify/classifier.py#L1938 This could just be left as is but potentially could be incorporate into logging somehow. |
…ith-machines/MapReader into kallewesterling/issue27
There was a problem hiding this comment.
A couple of comments from me.
I also think before we merge this we should make sure it works to show logs in notebooks as most people use mapreader this way.
One other comment is I have done some changes for the print if verbose method in my local clone but I think its diverged from your branch and I'm scared to push changes. I might make a branch from this branch and then PR into it to make sure it doesn't break anything
broken_files.txt
Outdated
There was a problem hiding this comment.
I think this is an accidental commit (maybe we should add test outputs to gitignore?)
There was a problem hiding this comment.
should these methods be deleted or is this a merge thing?
There was a problem hiding this comment.
likewise re. test output but shouldn't be a committed file
tests/sample_files/cropped_L.tif
Outdated
Summary
Fixes #27
Describe your changes
I have changed over the codebase's
printstatements intologgercommands instead. We might still need some instructions on how folks can get access to the logger, and see outputs (depending on level).Checklist before assigning a reviewer
Add testsUpdate relevant docsReviewer checklist
Please add anything you want reviewers to specifically focus/comment on.