-
Notifications
You must be signed in to change notification settings - Fork 0
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
Log a warning when an executable is found, but does not have the exec… #22
Conversation
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences🚀 Don’t miss a bit, follow what’s new on Codacy. Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
Pull Request Test Coverage Report for Build 10078911247Warning: This coverage report may be inaccurate.This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.
Details
💛 - Coveralls |
Fixes #21. |
I'm uncomfortable with the cross dependency to another part of the snippets package-- even if it is locked away and not top level. I need to check, but if we import the logger and then just warnings.warn (or maybe (re)import the default python logger and pass a warning there) doesn't this get caught by the pyiron log? I thought it should bz virtue of the logger module configuring the base logger, but maybe I'm being naive about a fresh instance of the logger ignoring this |
Fair, I think this should be possible, but will need to check. |
If it doesn't work pretty straightforwardly, I am content to make an exception (especially since it's not a top-level import), but let's check if we can have our cake and eat it too. |
The standard library has this, with the caveat that it redirects all warnings and to a hard coded logger, but it would be possible to setup similar logic in snippets.logger and redirect all warnings or warnings of a certain type to the pyiron logger. The warning type would have to be defined in a shared module somewhere so still link logger and resources. |
Coverage summary from CodacySee diff coverage on Codacy
Coverage variation details
Coverage variation is the difference between the coverage for the head and common ancestor commits of the pull request branch: Diff coverage details
Diff coverage is the percentage of lines that are covered by tests out of the coverable lines that the pull request added or modified: See your quality gate settings Change summary preferences🚀 Don’t miss a bit, follow what’s new on Codacy. Codacy stopped sending the deprecated coverage status on June 5th, 2024. Learn more |
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.
Looks good to me
That's unfortunate. But now playing with it myself and thinking about it more, I really, really don't like invoking the snippets logger here. Anyone hitting this warning will suddenly get a An alternative would be something like sticking a |
Avoids creating random files for users who otherwise do not use the logger.
Agree about the uncleanliness when not using the logger otherwise, so I reverted back to warning. I've added a custom warning type, so that we could potential filter and redirect it to the logger in the future, but I'm not too married to it. |
… bit set