Use Python logging system for emitting log messages when running via the Python API #159
Conversation
…the Python API Fixes #141
|
@cbrnr Care to test if this works? I don't think the logger honors existing tqdm output, but maybe this is not a problem – I haven't tested it yet |
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
This comment was marked as resolved.
Problems:
|
|
The token messages also appear on |
I don't know. I would believe it also appears in |
|
OK, so the problems you listed (at least the first two) are not related to this PR and should probably fixed in another PR. |
|
I believe so. Any help in verifying and tracking this would be appreciated! |
|
Nice! Once we have a changelog message, we can merge |
| message will shop up when running from the CLI and the Python API. | ||
|
|
||
| """ | ||
| from openneuro import _RUNNING_FROM_CLI # avoid circular import |
There was a problem hiding this comment.
Rather than this it seems a tiny bit cleaner to have a global var in this file. Then here you don't need any import and above...
| @@ -61,6 +61,8 @@ def download_cli( | |||
| ] = 5, | |||
| ) -> None: | |||
| """Download datasets from OpenNeuro.""" | |||
| openneuro._RUNNING_FROM_CLI = True | |||
There was a problem hiding this comment.
You can have this:
| openneuro._RUNNING_FROM_CLI = True | |
| openneuro._logging._RUNNING_FROM_CLI = True |
(along with an import openneuro._logging at the top)
|
For people using it as a module I think this would be useful, so despite the added complexity I'd still go for it. I'd also apply Eric's suggestion. |
Fixes #141
Adds a
cli_onlykwarg to emit some log messages only when running from the CLI.