eclipse-cyclonedds / cyclonedds-python

Other
54 stars 44 forks source link

Is SampleInfo really an internal interface? #179

Closed mlamby closed 1 year ago

mlamby commented 1 year ago

An instance of the SampleInfo (cyclonedds.internal.SampleInfo) class is dynamically added to the samples returned by DataReader (`cyclonedds.sub.DataReader).

Is the SampleInfo class really meant to be an internal type considering it is added to every sample?

I like to add an assert to help out the intellisense and it seems wrong to have to import from cyclonedds.internal to get access to the SampleInfo class.

e = reader.take_one()
si = e.sample_info
assert(isinstance(si, cyclonedds.internal.SampleInfo))
# Now do stuff with the si instance with full intellisense support.
thijsmie commented 1 year ago

Hi @mlamby,

It is indeed debatable whether this SampleInfo should be in Internal. However, since as a user you're never meant to construct one, just consume the type, I think it is justified in this case. It would be nice to be able to type-hint that the read/take() methods add this sampleinfo attribute, but I've failed to come up with the proper typing.Protocol to do this (or generic subclassing?) In any case, if you could somehow solve that problem then the whole issue with you needing to import it is also gone. If not, then you'll just have to live with the cyclonedds.internal import :)

mlamby commented 1 year ago

Thanks for the answer and that all makes sense so I will close the issue.

The python typing problem sounds like an interesting one to solve. For the moment though I will keep using the cyconedds.inernal import.

Thanks for the awesome project by the way!