[NEAT-237, NEAT-262] 🫠Adding Svein Harald Spreadsheets #459
Conversation
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
| @@ -143,6 +144,7 @@ def _create_metadata_from_model( | |||
| updated = now | |||
| return DMSMetadata( | |||
| schema_=SchemaCompleteness.complete, | |||
| data_model_type=DataModelType.solution if has_reference else DataModelType.enterprise, | |||
There was a problem hiding this comment.
This keeps thing simple
| @@ -111,17 +111,27 @@ def __init__( | |||
| self.required = required | |||
| self.metadata = metadata | |||
| self._sheet_prefix = sheet_prefix | |||
| self._seen_files: set[Path] = set() | |||
There was a problem hiding this comment.
is this for possibility to have partial/modular rules?
| def sheet_names(self, role: RoleTypes) -> set[str]: | ||
| names = MANDATORY_SHEETS_BY_ROLE[role] | ||
| return {f"{self._sheet_prefix}{sheet_name}" for sheet_name in names if sheet_name != "Metadata"} | ||
|
|
||
| def read(self, filepath: Path) -> None | ReadResult: | ||
| with pd.ExcelFile(filepath) as excel_file: | ||
| self._seen_files.add(filepath) |
There was a problem hiding this comment.
I see that you expect that this is support use case when you split Metadata, Classes, ... into individual files, but not use case where we have portion of data model (and all underlaying sheets) in multiple files.
Maybe tech debt task?
| if self.metadata.schema_ is SchemaCompleteness.partial: | ||
| return self.issue_list | ||
| dms_schema = self.rules.as_schema() | ||
| self.issue_list.extend(dms_schema.validate()) |
There was a problem hiding this comment.
Is this former _validate_schema?
There was a problem hiding this comment.
Yes, since it turned into a one liner, I thought it was overkill to have a function for it.
| @@ -213,14 +220,7 @@ def _validate_extension(self) -> None: | |||
| ) | |||
| ) | |||
|
|
|||
| def _validate_performance(self) -> None: | |||
There was a problem hiding this comment.
renamed to _validate_filter, a bit misleading name as what we are checking is if filter is applied to one too many containers for the same view
| self._consistent_container_properties() | ||
|
|
||
| self._referenced_views_and_containers_are_existing() |
There was a problem hiding this comment.
Should we do any check if SchemaCompleteness is partial ?
There was a problem hiding this comment.
I think this method will break if .last is missing?
There was a problem hiding this comment.
I have this:
if not self.rules.last:
raise ValueError("The schema is set to 'extended', but no last rules are provided to validate against")
| self._consistent_container_properties() | ||
|
|
||
| self._referenced_views_and_containers_are_existing() |
There was a problem hiding this comment.
I think this method will break if .last is missing?
|
Issue with GitHub Action, ran tests locally and merging to avoid being blocked. |
Discovered that these sheets were missing in addition to a set of bugs.
Loading Svein Harald's sheet turned out to completely break the validation of DMS, so had to do NEAT-237 as well.
Realize that there are several inconsistencies in the code wrt to the metadata properties. Suggest the following:
DataModelType this determines how we validate against the reference sheets.
SchemaCompleteness This determines the overall consistency checks for the Data Model. This is connected to the Last sheets.
ExtensionType:
Note New solution model should set their schema to
complete. The schema as a whole will still include the reference model.