-
Notifications
You must be signed in to change notification settings - Fork 23
Logging w/ consolidation of verbosity kwargs & attrs #82
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
Logging w/ consolidation of verbosity kwargs & attrs #82
Conversation
Note: setup and configuration is identical to that in cfdm, except cf-python uses its own LOG_LEVEL constant in the CONSTANTS dictionary to manage the log level (not the one from cfdm). Includes loosening of the cfdm version contraint from an exact to a minimum version, as agreed previously (changed now for development).
Includes application to aggregate module via _Meta class. Tests to come in next commit.
It may not be workable in these cases due to complications so we can use the global LOG_LEVEL to manage these instead (at least for this release).
From testing it is not simple to add verbosity as an instance attribute where it was not an equivalent attribute before (e.g. self.info) due to coupling etc. Abadon idea to extend. The global cf.LOG_LEVEL can manage message filtering inside those classes.
|
I will address the two outstanding tasks while doing the release & awaiting the tests to complete for that. Opening now to see how bad the merge conflicts are via the UI. Then I will see how the CI tests fare. I am seeing a sub-test error locally ( |
|
Conflicts resolved, now closing & re-opening to force the GH Actions checks to start. |
e0dc34f to
5e957af
Compare
|
Trying the CI checks again, & I have added an install of |
|
This PR has now become muddied with some pre-release commits, notably PEP8 style fix lines & UM read tweaks to try to get some of the test failures to pass. I should really have done these post-PR but I did not think ahead. Running the tests again, as I was trying to pinpoint which test was giving a full debug log dump & think I have sorted that (it was due to updates that had gone into master & needed amending for the logging logic after incorporation here). |
|
Final check before merge. I expect the test_PP_* tests to fail, as there is clearly an issue with the GH Actions environment/setup in this respect. Those tests pass locally so it is fine. |
|
Just the three |
Closes #37. Sister PR to NCAS-CMS/cfdm#34, with largely the same purpose & implementation, & using much of the corresponding functionality there (by import where possible) but with the following key differences:
cf-pythonappliesinfo&_debugkeywords as well as justverboseto enable filtering on some level of verbosity. These keywords have all been converted toverbosefor consistency, & appropriate level have been given to any call being filtered as such, e.g:has been changed to the following, so it only emerges with debug logging configured locally via
verbosekeyword (see next point), or if not globally:where
infolevels have been mapped appropriately, as discussed, namely:infolevelverboselevelunlike in
cfdmthere are cases where the verbosity is specified not via a function keywords arguments, but via an instance attribute e.g.self.verbose(or, pre-conversion,self.info). To manage these I have created another decorator which is analagous to the existing_manage_log_level_via_verbositybut adapts the log level for runs of decorated methods according to the value of theverboseas an attribute instead as a keyword argument.Work still TODO for this PR
Currently a WIP with functionality largely there but docs & testing pending:
info&_debugwhere appropriate;assetLog-based tests;cfdmlogging);