Input format for components #31
Conversation
vargastat
force-pushed
the
feature/components_yaml
branch
from
6a3c638 to
7bf7637
Compare
Signed-off-by: Sylvain Leclerc <[email protected]>
vargastat
force-pushed
the
feature/components_yaml
branch
from
7bf7637 to
aff0de8
Compare
src/andromede/study/data.py
Outdated
| try: | ||
| return pd.read_csv(ts_path, header=None, sep="\s+") | ||
| except FileNotFoundError: | ||
| raise FileNotFoundError(f"File '{timeseries_name}' does not exist") |
There was a problem hiding this comment.
I'm not sure it's a good thing to add this exception try/catch. Your error message brings less info than the original one. If you want to keep this mechanism, please print at least the full file path with extension aka ts_path.
There was a problem hiding this comment.
yes you are right, many things can go wrong I but Exception instead and whole file_path and name in the message
| return InputComponents.model_validate(tree["study"]) | ||
|
|
||
|
|
||
| # Design note: actual parsing and validation is delegated to pydantic models |
There was a problem hiding this comment.
Note : C++ has no pydantic. What does this do for us ?
There was a problem hiding this comment.
Indeed, pydantic is used here to convert from a dictionary/list representation to object oriented representation, including validation of inputs.
Depending on available libs on c++ side, we will most likely need to implement more things by ourselves (c++ does not have reflection so it cannot really automate this kind of object construction, unlike python/java).
| if path_to_file is not None and timeseries_name is not None: | ||
| timeseries_with_extension = timeseries_name + ".txt" |
There was a problem hiding this comment.
Bit strange. Why add the prefix here ?
I understand that we don't want ".txt" in the yaml files, but this adds implicit knowledge (the file XXX.txt must be referenced as XXX in the yaml).
There was a problem hiding this comment.
We want to hand different extensions and that the final user should only put timeseries file name in the .yaml file
| @pytest.fixture | ||
| def input_component( | ||
| data_dir: Path, | ||
| ) -> InputComponents: | ||
| compo_file = data_dir / "components.yml" | ||
|
|
||
| with compo_file.open() as c: | ||
| return parse_yaml_components(c) | ||
|
|
||
|
|
||
| @pytest.fixture | ||
| def input_library( | ||
| data_dir: Path, | ||
| ) -> InputLibrary: | ||
| library = data_dir / "lib.yml" | ||
|
|
||
| with library.open() as lib: | ||
| return parse_yaml_library(lib) |
There was a problem hiding this comment.
There should be separate directories, with possibly multiple files.
study/
├── components
├── libraries
└── timeseries
There was a problem hiding this comment.
it is only for tests purposes because in Python it is difficult to resolve absolut paths in test mode but in real scenario the file directories you describe are OK
|
New pull request will be created |
Proposition of input format for components and parameter values.