Skip to content

Update logging #92

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

Merged
merged 4 commits into from
Apr 12, 2022
Merged

Update logging #92

merged 4 commits into from
Apr 12, 2022

Conversation

cehbrecht
Copy link
Collaborator

Pull Request Checklist:

  • This PR addresses an already opened issue (for bug fixes / features)
    • This PR fixes issue #xyz
  • Tests for the changes have been added (for bug fixes / features)
  • Documentation has been added / updated (for bug fixes / features)
  • HISTORY.rst has been updated (with summary of main changes)
  • I have added my relevant user information to AUTHORS.md
  • What kind of change does this PR introduce?:

This PR updates the logging to use loguru.
This change was introduced in clisops>=0.9.0, see roocs/clisops#216

  • Does this PR introduce a breaking change?:

  • Other information:

@cehbrecht cehbrecht requested a review from Zeitsperre April 12, 2022 13:02
@cehbrecht
Copy link
Collaborator Author

cehbrecht commented Apr 12, 2022

@Zeitsperre this is mainly a copy+paste from roocs/clisops#216. We can probably reduce some duplicates, like the tests._common.ContextLogger.

@Zeitsperre
Copy link
Contributor

@cehbrecht Thanks and looks good! I'm hoping that this new engine solves a few of our problems.

If pytest supported class fixtures, the ConextLogger would be much better off as one. I've written a similar one into xclim (https://github.com/Ouranosinc/xclim/blob/9acbc72e8a22d3267929db3b88080c12b9f01288/xclim/testing/_utils.py#L35) that handles for the behaviour of loguru being enabled when xclim's version ends in beta.

If it can eventually be defined in a lower-level library (like, daops I suppose), we could then remove redundant definitions elsewhere.

@Zeitsperre
Copy link
Contributor

@cehbrecht #93 should be compatible after the merge of this PR.

@cehbrecht cehbrecht merged commit 58805b1 into master Apr 12, 2022
@cehbrecht cehbrecht deleted the update-logging branch April 12, 2022 14:58
@cehbrecht cehbrecht mentioned this pull request Apr 12, 2022
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants