Introduce ruff formatter#601
Conversation
All applied rules without violations
90c4473 to
bea02d4
Compare
d753f24 to
371469d
Compare
|
|
||
| __all__ = [ | ||
| "AASFromJsonDecoder", | ||
| "StrictAASFromJsonDecoder", | ||
| "StrictStrippedAASFromJsonDecoder", | ||
| "StrippedAASFromJsonDecoder", | ||
| "read_aas_json_file", | ||
| "read_aas_json_file_into", | ||
| "AASToJsonEncoder", | ||
| "StrippedAASToJsonEncoder", | ||
| "object_store_to_json", | ||
| "write_aas_json_file", | ||
| ] |
There was a problem hiding this comment.
Are we sure this "all" really is all? The reason I ask is that we might not use everything from the star import. but if this is not everything public from this module, then we might break other people's code that relies on import adapter.json.
| __all__ = [ | ||
| "AASFromXmlDecoder", | ||
| "StrictAASFromXmlDecoder", | ||
| "StrictStrippedAASFromXmlDecoder", | ||
| "StrippedAASFromXmlDecoder", | ||
| "XMLConstructables", | ||
| "read_aas_xml_element", | ||
| "read_aas_xml_file", | ||
| "read_aas_xml_file_into", | ||
| "object_store_to_xml_element", | ||
| "object_to_xml_element", | ||
| "write_aas_xml_element", | ||
| "write_aas_xml_file", | ||
| ] |
There was a problem hiding this comment.
Same as above, qre we sure this "all" really is all? The reason I ask is that we might not use everything from the star import. but if this is not everything public from this module, then we might break other people's code that relies on import adapter.xml.
There was a problem hiding this comment.
So I checked again and your suspicion was correct. With the current __all__ we would lose some functionality / side effect:
E.g.:
from basyx.aas.adapter.xml import *
xml_deserialization.SomeThing # will break, worked beforePrevious implementation worked because the imports added the namespaces as a side effect. Now only the actually listed namespaces are exposed.
Although it differs, the current implementation exposes all of the intended public API I think. It would only break in cases where other people's code tries to use the namespaces xml_deserializationand xml_serialization after the star import. It is still possible to use from basyx.aas.adapter.xml import xml_deserialization and go from there.
So I dont think this is really breaking, maybe actually cleaner? What do you think?
There was a problem hiding this comment.
I absolutely think that this is cleaner. Maybe we can do this and add a small note to the release notes.
TODO:
Manually introduce (PYI) - typing best practices