Export PdoMap and PdoVariable classes in pdo module #464
Conversation
With commit 0285f58, the Map class was inadvertently removed from the imports directly available within the canopen.pdo package. However, it is referenced as canopen.pdo.PdoMap in the p402 module and documentation. Restore that import using the new names PdoMap and PdoVariable. Move the compatibility define to the very bottom, just like in other modules. Provide an __all__ listing to clarify what constitutes the public API.
Replace the sphinx parameter type documentation with a proper type hint annotation. Same for the tpdo_pointers and rpdo_pointers attributes.
acolomb
changed the title
Codecov ReportAttention: Patch coverage is
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## master #464 +/- ##
==========================================
+ Coverage 67.20% 67.23% +0.03%
==========================================
Files 26 26
Lines 3095 3098 +3
Branches 522 522
==========================================
+ Hits 2080 2083 +3
Misses 873 873
Partials 142 142
|
|
|
||
|
|
||
| # Compatibility | ||
| Variable = PdoVariable |
There was a problem hiding this comment.
I wonder if Variable should be listed in __all__. In principle __all__ contains the entire public API of the module. However, it's not very common to list objects defined in the file in __all__ is it?
There was a problem hiding this comment.
I don't know what's common practice with regard to that __all__ attribute. But my understanding is that it controls what from ... import * does. In that case, Variable should not be imported, as it is a deprecated alias only kept for compatibility. But if someone references it directly, that still works.
There was a problem hiding this comment.
Svein:
However, it's not very common to list objects defined in the file in
__all__is it?
That's of course possible; for example, a sentinel or a singleton could also be a part of a public API.
André:
[...] my understanding is that it controls what
from ... import * does.
[...] But if someone references it directly, that still works.
Both are correct. from canopen.pdo import Variable still works.
4 participants
With commit 0285f58, the
Mapclass was inadvertently removed from the imports directly available within thecanopen.pdopackage. However, it is referenced ascanopen.pdo.PdoMapin thep402module and documentation.Restore that import using the new names
PdoMapandPdoVariable. Move the compatibility define to the very bottom, just like in other modules. Provide an__all__listing to clarify what constitutes the public API.