Conversation
First version of the config parser
| return res | ||
| else: | ||
| return func, var_names | ||
| # Evaluate directly if no variables are found |
There was a problem hiding this comment.
Probably better to have the following part in a separate function like _eval that only does the evaluation once everything is set.
There is also some update in the example config file for Lensfit
| config_path=None, | ||
| ): | ||
|
|
||
| if isinstance(config_path, type(None)): |
There was a problem hiding this comment.
| if isinstance(config_path, type(None)): | |
| if config_path is None: |
| if not os.path.exists(config_path): | ||
| raise ValueError(f"No file found at: {config_path}") | ||
|
|
||
| config_raw = self._read_yaml_file(config_path) |
There was a problem hiding this comment.
Quand tu fais ça tu ne t'autorises pas à utiliser directement un dictionnaire de config en input, il faut absolument passer par un fichier YAML externe.
Une solution pour bypasser ça c'est d'utiliser une @classmethod from_yaml() qui comme son nom l'indique s'occupe de checker le path et de fournir la config en raw. La sortie de cette méthode est une instance de ta classe ConfigParser, qui prend donc en input une config_raw plutot qu'un path.
There was a problem hiding this comment.
Ou tu pourrais même filer un dictionnaire en entrée comme ce que tu retournes parse_config
There was a problem hiding this comment.
Je pense que tu t'es pas mal compliqué la vie et que tu pourrais simplifier, mais globalement
- la plupart de tes fonctions de la classe ConfigParser ne font meme pas appel à
selfce qui est un gros indice pour te dire que ces fonctions devraient sortir de la classe. - j'aurais tendance à séparer les fonctions qui parsent des fonctions qui checkent. Ce qui n'empeche pas d'avoir des checks dynamiques pendant le parsing, mais qui ne les mélangent pas à des checks "statiques" qui peuvent être faits dès le début sur l'ensemble du dictionnaire comme la présence de certaines keys. 1) ça te permet de faire une simple boucle sur une liste de keys nécessaires (quitte à faire le ValueError avec la bonne key) 2) Et ça rend le parsing ensuite bien plus lisible.
- j'aurais tendance à séparer la classe de parser en sous-classes pour mieux gérer les subtilités, un peu comme ce que j'ai fait avec https://github.com/aboucaud/galcheat . Une pour les galaxy catalog, une pour les star catalogs, etc. Mais tu gardes un MainParser qui permet de les regrouper et d'y accéder facilement. Ce sera plus lisible et plus facilement testable.
Voilà pour un premier passage rapide.
| if not os.path.exists(config_path): | ||
| raise ValueError(f"No file found at: {config_path}") | ||
|
|
||
| config_raw = self._read_yaml_file(config_path) |
There was a problem hiding this comment.
Ou tu pourrais même filer un dictionnaire en entrée comme ce que tu retournes parse_config
|
Also put this file under the |
2 participants
First version of the config parser.
See issue #1 for details.
This PR will be updated as features are added to the library.