jordanperr
commented
1 year ago This pull request looks like it goes towards the right direction in separating out the analysis schemas and providing easier-to-use JSON and YAML files for people interested in reading the schemas. But I don't think it actually closes #229. The point of #229 was to remove the static analysis requirements altogether, providing some sort of "AnalysisMethod.generateSchema()" method which generates the required data given the analysis parameters. Perhaps you meant to close #227 with this PR, which is more directly related to this PR? However, it also doesn't completely close #227, since there are still some constants in the code.
From a technical perspective, I think this looks fine. I like that the Metadata objects have been broken out of plant.py and placed in their own module. I also like how the RST documentation reads some parts of the schema programmatically.
Couple questions:
-
How do you intend to keep the schema JSON and YAML files up to date? Perhaps we could re-generate them in a git pre-commit hook? Or have Sphinx execute schema.py from sphinx/conf.py before building the docs? I'm still not convinced we should even commit the JSON and YAML files in the repository, since they are not the original source of the information. We could exclude them from the repo (using gitignore) and then Sphinx could simply re-generate them on demand.
-
There are still some hard-coded tables in the sphinx documentation, immediately preceding the yaml. (For example, line 98 in schema.rst) How will this static table stay synchronized with the schema?
codecov-commenter
This PR addresses the themes of #229 with the following updates:
shema/subpackage that contains all of the meta data classes, analysis requirements, and data specifications to declutteropenoa/plant.py.PlantDataobject and all of the currently defined base analysis schematics along with a method to regenerate these specifications as the model internals get updated.PlantData, meta data, and schema objects.self.plant.validate()to the initialization steps of all analysis classes so that a user can't bypass the validation process.python openoa/schema/schema.py.WTUR_TurNamhas been updated toasset_idAddress the core aspects that would enable #229, and most of #227.