Added log file to isofit run cli#733
Conversation
|
Thanks Evan, this is a nobrainer for me...while you're in here, want to hook up an option for the log level too? Once checks pass, looks good. |
|
@pgbrodrick I'm pretty sure it has the log level already, just not the file. I haven't tested to double check that the log level is working properly however. |
|
Yup, you're right, I didn't scroll up on the diff - looks like it's all connected. |
jammont
left a comment
There was a problem hiding this comment.
Looks good! Thanks for implementing some missing functionality. Good to know this command is even used 👍
| default="INFO", | ||
| ) | ||
| def cli(config_file, level): | ||
| @click.option("--log_file", default=None) |
There was a problem hiding this comment.
default=None is redundant as that's already the default
|
|
||
| logging.basicConfig(format="%(message)s", level=level) | ||
| Isofit(config_file=config_file, level=level).run() | ||
| logging.basicConfig(format="%(message)s", filename=log_file, level=level) |
There was a problem hiding this comment.
I know you didn't add this line, but the Isofit class also calls logging.basicConfig, maybe we should simplify and just remove this line? Can't think of a reason this one would be needed, and the only difference is Isofit's include additional formatting.
Just to note, logging.basicConfig ignores any subsequent calls, so this one takes priority.
There was a problem hiding this comment.
Perfect, thanks for clarifying!
I had the need for log files from the
isofit runcli while testing the wavelength calibration. Figured I roll into a PR.