Use logging #129
Use logging #129
Conversation
| @@ -39,12 +44,11 @@ def save_default_train_options(path_save: str) -> None: | |||
| Save path for default training options json. | |||
|
|
|||
| """ | |||
| if os.path.exists(path_save): | |||
| print('Training options file already exists:', path_save) | |||
| path_save = Path(path_save) | |||
There was a problem hiding this comment.
You may want to make this Path(path_save).expanduser().resolve(). It will make it so people can put relative paths and also include their home directory like ~/Desktop/trained_fnet_model/
There was a problem hiding this comment.
Cool! I'll look to add this type of functionality when I address #128.
| 'seed': None, | ||
| } | ||
| with open(path_save, 'w') as fo: | ||
| with path_save.open('w') as fo: |
There was a problem hiding this comment.
pathlib.Path works with the default open function, but TIL Path objects have an open function too.
| print('Training options file already exists:', path_save) | ||
| path_save = Path(path_save) | ||
| if path_save.exists(): | ||
| logger.info(f'Training options file already exists: {path_save}') |
There was a problem hiding this comment.
I'd probably throw a custom exception here personally.
There was a problem hiding this comment.
An exception would imply that something went "wrong" though. The idea is to simply exit if the intended action had already been completed in the past. That way the user does not have to manually check the file system.
fnet/cli/predict.py
Outdated
| @@ -213,7 +217,7 @@ def save_args_as_json(path_save_dir: str, args: argparse.Namespace) -> None: | |||
| ) | |||
| with open(path_json, 'w') as fo: | |||
| json.dump(vars(args), fo, indent=4, sort_keys=True) | |||
| print('Saved:', path_json) | |||
| logger.info('Saved: {path_json}') | |||
There was a problem hiding this comment.
Why is the logger statement indented into the open statement?
There was a problem hiding this comment.
Originally, the intent was to only log a message if a json was actually written. Now that I think about it though, if a json was not written because of, say, and IO error, then there would be some exception and the program would crash out anyway. Anyway, I'll fix this. :)
There was a problem hiding this comment.
I added another commit to pull the logging statement outside of the context block.
This PR converts the package print statements into logging module calls. Addresses #121.
Also, fixes bug #125.