modern path handling and pydantic implementation#82
Draft
benatouba wants to merge 3 commits intocryotools:developmentfrom
Draft
modern path handling and pydantic implementation#82benatouba wants to merge 3 commits intocryotools:developmentfrom
benatouba wants to merge 3 commits intocryotools:developmentfrom
Conversation
Transform all path variables to the more modern `pathlib.Path` object. This helps with cross-platform compatibility and makes the code more readable. Some type annotations were also passed. Meant to be applied in conjunction with a move to Pydantic for configuration validation. NOTE: If cherry-picked, needs to be revised carefully as the commit is meant to be applied with a move to Pydantic. Fixes cryotools#81
benatouba
force-pushed
the
development
branch
from
69335f0 to
6e4fe41
Compare
Fail early, fail gracefully. Implements config validation via pydantic for CosipyConfig (main_config), SlurmConfig (slurm_config) and Constants (constants_config, still needs more detailed model description). NOTE: Other configs are still missing. This provides config validation similar to dataclasses but more elaborate. The validation at runtime allows us to check early on if any config is missing or invalid and stops the model with informative error messages. Furthermore pydantic provides us type information, which makes it easier to extend the code as the project gets larger. While implementing this, I already found that the previous `Constants` class was missing some entries that are in the `constants.toml` file. Flattening the config structure (as done before) is now done before model validation by use of pydantics `@model_validator` decorator. NOTE: This is supposed to be a first draft and further details should be implemented. IMPORTANT: Look for NOTE, FIXME, and HACK comments in the code.
benatouba
force-pushed
the
development
branch
from
6e4fe41 to
4206a1e
Compare
Collaborator
|
How would you feel about splitting this into two separate pull requests, one with your pydantic classes for #81, and one with your better pathing type safety for #80? Would it also be possible for you to launch pull requests from individual feature branches from your repository, rather than from your development branch? This means others can edit your commits without having to overwrite your own development branch (and potentially breaking things there). |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #80
Fixes #81
The aim of this pull request is mainly to simply further development and enhance user experience, at setup and running of the model.
Please look at the commits and commit messages and give your thoughts.
Description in commit messages.
Work needed:
All tests are passing on
Python3.12