-
-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Performed changes to listed warn() and logger.info #13530
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
base: main
Are you sure you want to change the base?
Conversation
/logger.debug as listed in POSSEE Conducted changes to non-logging messages using grep calls in POSSEE document, many were false positive. actual changes listed below.
|
Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴 |
| return cov | ||
|
|
||
| logger.info(" Did not find the desired covariance matrix (kind = %d)", cov_kind) | ||
| logger.info(f" Did not find the desired covariance matrix (kind = {cov_kind})") |
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.
This one I think was okay in main. These are logging.logger calls so delayed repr calls using % and *args can be useful
mne/_fiff/meas_info.py
Outdated
| # This shouldn't happen if we're up to date with the FIFF | ||
| # spec | ||
| warn(f"Discarding extra channel information kind {kind}") | ||
| warn("Discarding extra channel information kind %s" % (kind,)) |
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.
@drammock I think these we probably don't want to change... our warn function is really a wrapper around warnings.warn which doesn't use *args or delayed % formatting
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.
So we only want % for errors and for logger.info and logger.debug, but not for warn()?
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.
Yeah that's the idea
|
@larsoner i will push commits to fix style next |
for more information, see https://pre-commit.ci
reverted back to f-string per converesation
for more information, see https://pre-commit.ci
/logger.debug as listed in POSSEE
Conducted changes to non-logging messages using
grep calls in POSSEE document, many were false
positive. actual changes listed below.
Reference issue (if any)
What does this implement/fix?
Additional information