| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
There was a problem hiding this comment.
@bruno-f-cruz thanks, looks good overall. I have a few questions / comments:
Sorry, something went wrong.
| @staticmethod | ||
| def from_url( | ||
| url: str, | ||
| base_path: Optional[PathLike] = None, |
There was a problem hiding this comment.
base_path is indeed getting clunky, I'm slightly uncomfortable of having different behaviors depending on which function gets called here, so maybe just getting rid of it would be best.
Sorry, something went wrong.
There was a problem hiding this comment.
Not sure I follow. The use of base_path is consistent across methods. If provided it will try use it as a root directory where all files are expected to be found. If not provided it will assume either the current working directory (e.g. from_url) or the root of the provided yml (e.g. from_file). Are these two defaults what are raising flags for you? Happy to go with a different one if you want. We can also just throw an error at the level of the DeviceReader class if no path was provided and the user try to use the no-input overload of the register load function.
Sorry, something went wrong.
| return DeviceReader(device, reg_readers) | ||
|
|
||
| @staticmethod | ||
| def from_str( |
There was a problem hiding this comment.
Still looking for a better name:
Sorry, something went wrong.
There was a problem hiding this comment.
from_schema makes me nervous as people might think you should pass the actuall schema file (i.e. .yml). Would from_string be better (ToString is also a thing after all haha)?
Sorry, something went wrong.
|
@glopesdev lets review #28 and #27 first and then we will handle this one. |
Sorry, something went wrong.
…harp-tech/harp-python into feat-device-reader-constructors
|
@glopesdev I am taking a second look at this and I think we need to mainly decide on a few points:
|
Sorry, something went wrong.
This allows `g` aka `_create_register_reader` to take care of the path inference logic
Make `data` input to `_compose_parser` optional
Replace license classifier with SPDX expression
…harp-tech/harp-python into feat-device-reader-constructors
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Closes #25