picas command line helper and general cleanup - #41
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a command line interface for Picas and performs general project cleanup. The PR adds essential infrastructure for making Picas a proper command-line tool with configuration management capabilities.
- Adds a
setup.pyfile with console script entry point for CLI usage - Introduces configuration management through
PicasConfigclass with YAML-based configuration - Creates command line interface with
initandpasswdsubcommands - Performs code formatting improvements in example files
Reviewed Changes
Copilot reviewed 9 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| setup.py | Defines package setup with CLI entry point and package structure |
| picas/metadata.py | Contains package metadata including version, authors, and project information |
| picas/picas_config.py | Implements configuration management with YAML schema validation |
| picas/apps/picas.py | Provides command line interface with argument parsing and subcommands |
| examples/*.py | Code formatting improvements and todo comments for future enhancements |
| Makefile | Basic project automation with clean and help targets |
| description = "Python client using CouchDB as a token pool server." # import from the toml file | ||
| project_no_spaces = project.replace(' ', '') | ||
| version = '1.1.0.dev0' # import from the toml file | ||
| authors = ["Jan Bot, Joris Borgdorff, Lodewijk Nauta, Haili Hu"] # import from the toml fil |
There was a problem hiding this comment.
The comment has a typo: 'fil' should be 'file'.
| authors = ["Jan Bot, Joris Borgdorff, Lodewijk Nauta, Haili Hu"] # import from the toml fil | |
| authors = ["Jan Bot, Joris Borgdorff, Lodewijk Nauta, Haili Hu"] # import from the toml file |
| Load the configuration yaml file from the specified path. | ||
| """ | ||
| print(f"Loading configuration from {self.config_path}") | ||
| if not os.path.exists(self.config_path): |
There was a problem hiding this comment.
The path expansion should be applied before checking if the file exists. Use os.path.expanduser(self.config_path) instead of self.config_path in the exists check.
| if not os.path.exists(self.config_path): | |
| if not os.path.exists(os.path.expanduser(self.config_path)): |
| def save_config(self, args): | ||
| """ | ||
| Save the current configuration to the specified path. | ||
| """ | ||
| # Implementation to save the configuration file | ||
| print(f"Saving configuration to {self.config_path}") No newline at end of file |
There was a problem hiding this comment.
The method has no implementation and unclear parameter purpose. The docstring should explain what args parameter contains and the method should have a proper implementation or be marked as TODO.
| def save_config(self, args): | |
| """ | |
| Save the current configuration to the specified path. | |
| """ | |
| # Implementation to save the configuration file | |
| print(f"Saving configuration to {self.config_path}") | |
| def save_config(self): | |
| """ | |
| Save the current configuration (`self.config`) to the configuration file at `self.config_path`. | |
| """ | |
| print(f"Saving configuration to {self.config_path}") | |
| with open(os.path.expanduser(self.config_path), 'w') as fobj: | |
| yaml.safe_dump(self.config, fobj) |
| Function that initializes the picas configuration | ||
| """ | ||
| print('Initializing picas configuration...') | ||
| picas_config = PicasConfig() |
There was a problem hiding this comment.
The function accepts parsed_args parameter but doesn't use it. The PicasConfig should be initialized with the config path from parsed_args: PicasConfig(parsed_args.config_path).
| picas_config = PicasConfig() | |
| picas_config = PicasConfig(parsed_args.config_path) |
| @@ -1,3 +1,4 @@ | |||
| # .. todo Mher:: add a proper argument parser | |||
There was a problem hiding this comment.
[nitpick] The comment appears to have a typo or unclear reference with 'Mher'. Consider clarifying or correcting this reference.
| # .. todo Mher:: add a proper argument parser | |
| # TODO: add a proper argument parser |
| @@ -1,3 +1,4 @@ | |||
| # .. todo Mher:: add a proper argument parser | |||
There was a problem hiding this comment.
[nitpick] The comment appears to have a typo or unclear reference with 'Mher'. Consider clarifying or correcting this reference.
| # .. todo Mher:: add a proper argument parser | |
| # TODO: add a proper argument parser |
|
These changes were included in #42, close without merging |
No description provided.