add -v cmdline flag to pyvisa-shell for warn/info/debug logging output #816
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #816 +/- ##
=======================================
Coverage 79.55% 79.55%
=======================================
Files 27 27
Lines 5380 5380
Branches 515 515
=======================================
Hits 4280 4280
Misses 1073 1073
Partials 27 27
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
pyvisa/cmd_line_tools.py
Outdated
| @@ -43,6 +51,11 @@ def visa_main(command: Optional[str] = None) -> None: | |||
| subparsers.add_parser("shell", help="start the PyVISA console") | |||
|
|
|||
| args = parser.parse_args() | |||
| logging.basicConfig( | |||
| level={0: logging.ERROR, 1: logging.WARN, 2: logging.INFO, 3: logging.DEBUG}[ | |||
There was a problem hiding this comment.
any reason for an int-indexed dict instead of a tuple/list?
There was a problem hiding this comment.
None at all! A tuple or list would be more compact, no less explicit, & works just as well.
There was a problem hiding this comment.
Can you address this then ?
|
Good call on making logging easier to enable here. I think that |
|
One issue with making WARN default is that it changes the current behavior. If we're willing to change the current behavior, then I'm fine with that (and would kinda prefer it, so users get useful advice without having to figure out there's a -v option in the first place). Another consideration is whether we want to add a command line option that decreases the verbosity level back to ERROR (presumably the default now), if users don't want to see WARN-level messages at all? (Seems like WARN level messages shouldn't be too annoying, though...) |
This is exactly why I think
This is fair, |
pyvisa/cmd_line_tools.py
Outdated
| @@ -43,6 +51,11 @@ def visa_main(command: Optional[str] = None) -> None: | |||
| subparsers.add_parser("shell", help="start the PyVISA console") | |||
|
|
|||
| args = parser.parse_args() | |||
| logging.basicConfig( | |||
| level={0: logging.ERROR, 1: logging.WARN, 2: logging.INFO, 3: logging.DEBUG}[ | |||
There was a problem hiding this comment.
Can you address this then ?
|
ping @jllanfranchi could you address the small change request and I will merge. |
6eebf65 to
1f6ef24
Compare
|
I took the liberty to rebase your PR and make the small changes we discussed. I will merge by the end of the week. |
46f6151 to
ee1b30b
Compare
for more information, see https://pre-commit.ci
This commit adds
-v(or-vvor-vvvor-v -vor-v -v -v) as command line flags that can be passed topyvisa-shellsuch that WARN (-v), INFO (-vv), or DEBUG (-vvv) messages will be emitted when running the shell. This can help users understand why a device is not listed that they believe should be listed when using that tool. Guidance in documentation can be added (by me, in the near future) to tell users to utilize these options if things are not working correctly. As it stands now,pyvisa-shelljust doesn't say anything, and the only way to get increased messages is to write a Python script to do this.This commit, along with pyvisa/pyvisa-py#423 in the pyvisa-py, (as soon as I also update documentation appropriately) will close #758 and I expect will also close #791.
serial_numbernot readable pyvisa-py#423 (TODO: update documentation)ruff check . && ruff format -c . --checkwith no errors