#104: Skip serialization of empty INI properties when configured #336
Conversation
…r mdu and to True for the rest of the files.
|
|
||
| def _serialize(self, _: dict) -> None: | ||
| config = SerializerConfig(skip_empty_properties=False) | ||
| write_ini(self._resolved_filepath, self._to_document(), config=config) |
There was a problem hiding this comment.
Visual Studio gives the following error for self._resolved_filepath:
Argument of type "Path | None" cannot be assigned to parameter "path" of type "Path" in function "write_ini" Type "Path | None" cannot be assigned to type "Path" Type "None" cannot be assigned to type "Path"
I think it is because _resolved_filepath is Optional and write_ini() expects a non-optional path.
There was a problem hiding this comment.
before this method is called, the _resolved_filepath property is checked for None, so it cannot be None here.
There was a problem hiding this comment.
Is that so? I see such a check only in ParsableFileModel._serialize(), but that will not be called if a subclass overrides that method, for example in INIModel._serialize().
I think the place that now makes sure everything runs well is this:
class FileModel(BaseModel, ABC):
# ...
def _save_instance(self) -> None:
if self.filepath is None:
Should we not add documentation that _serialize() should always only be called via save() and never directly?
There was a problem hiding this comment.
@priscavdsluis wrote that the latter is implicitly so, because _serialize() is a private function. That is true for end-user APIdocs. But amongst core-developers, it is good to know that other private functions should probably also not call _serialize directly. Maybe some one-liner docs in the function body?
There was a problem hiding this comment.
I added some docs and will close this PR.
There was a problem hiding this comment.
@priscavdsluis: almost done, please see one remaining comment.
|
Kudos, SonarCloud Quality Gate passed!
|
Fixes #104