Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Properties, shortcuts and different flavours of StreamInfo #4

Open
JanChec opened this issue Feb 22, 2018 · 1 comment
Open

Properties, shortcuts and different flavours of StreamInfo #4

JanChec opened this issue Feb 22, 2018 · 1 comment

Comments

@JanChec
Copy link

JanChec commented Feb 22, 2018

Hey!

Thanks for the lib, it's very helpful. I'd love if you used, in a next major version, properties instead of getters-almost-like-properties. For example I'd like to just write:
stream_info.channel_count
instead of:
stream_info.channel_count()
Simple addition of @property would work wonders :)

Moreover please don't use shortcuts in public methods. I saw one (maybe the only one) example: nominal_srate - I thought first, when doing CR for my colleague, that it was a typo. nominal_sampling_rate would be perfect, no reason to shorten it :)

While I'm at it, why:

original_stream_info = pylsl.resolve_streams()[0]
inlet = pylsl.StreamInlet(original_stream_info)
stream_info = inlet.info()

gives a StreamInfo instance with additional data? stream_info has channels data, while original_stream_info doesn't. We may be misusing the lib, but that's the way we get channels data for active streams, producing an obsolete inlet for that.

@cboulay
Copy link

cboulay commented Jul 22, 2022

I never noticed this post before. For others reading, please move discussions to https://github.com/labstreaminglayer/liblsl-Python
This repo should be archived.

To answer some questions:

While it would be nice to add properties, we can't get rid of the old getter methods without marking them as deprecated for 1-3 years to give users a chance to upgrade existing code. How do we mark them as deprecated and provide an upgrade path if the existing getter methods have the same name as the desired properties?

About shortening variable names, pylsl is a thin wrapper around liblsl. nominal_srate matches the name in liblsl. I guess we could have a slightly different variable name in pylsl than liblsl, but we could also keep it the same as the original library. Which is more straightforward for users and documentation purposes? Changing the variable name in the library is not an option at this point.

The original stream info not providing full header information is an annoyance. It has to do with the fact that the original stream info is populated only with information that is provided when outlets are doing their multicast advertising. When the info is used as part of an inlet, the inlet can then retrieve the full header information. We can't really change the information that goes into the broadcast, because this can be arbitrarily large (e.g., thousands of channels with labels, ranges, units, etc.).
Maybe the StreamInfo object can fill its contents on-the-fly when something not in the advertised header is requested? I think this would require establishing a connection with the outlet so it would incur quite a bit of overhead on the first check. Another downside is that this could potentially raise network errors that you wouldn't expect when fetching the property of an object but you might expect when creating an Inlet.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants