-
Notifications
You must be signed in to change notification settings - Fork 93
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
All methods/attributes can both be accessed directly through Session and a repeated capability #737
Comments
From Zen of Python and in favor of adding all_channels:
Against adding all_channels:
Also for: consistency with nidaqmx. TBD: What to do about nidmm. |
I think the fundamental problem is that we are dealing with an artifact of the C APIs: users can specify a channel string for both channel and device attributes, and the channel string means different things depending on the attribute types. For example, an empty string works for both channel and device attributes, but it means "all channels" for the former and "just access that attribute" for the latter. Unfortunately, this is not symmetric with non-empty channel strings, since the channel attribute will be happy but the device attribute will error. The same problem applies to functions. I see two approaches going forward: 1) live with the C API artifact or 2) organize attributes such that device attributes only appear in the session and channel attributes only appear in the channels object. I prefer 2, but I could live with 1 since practicality does indeed beat purity and because the root cause of the usage issue is the C API. I think we should decide based on this rule of thumb for implementing bindings: if there is a feature in the C API that is not pythonic, we should try to make the usage more pythonic by either exposing it better or just hiding it, but we should not make efforts to explicitly prevent it. Thus, whether we go with 1 or 2 depends on what is required to make the usage more pythonic. For example, if we have to add explicit checks or errors to prevent channel_count from being accessed with a channel string, then I feel like we are going against the rule of thumb and over engineering for purity-sake. However, if we can simply reorganize the attributes so that channel_count can't be accessed as a channel attribute, then I feel like that is the right direction for being pythonic. (This is probably what we would have done if we had written the driver API in Python from the ground up.) The difference I'm illustrating is admittedly subtle, as 2 may not be a simple task and may seem like over engineering in the long run. The point I'm getting at is that if 2 is indeed the best choice for being pythonic, then the design and implementation will naturally fall into place, and it will just feel right. With regard to NI-DMM and the issue of channels, we can implement 2 if we first answer the question: does attribute/function X fall under the 'channel' bucket or 'device' bucket? For example, if we determine that attribute X truly falls under the device bucket (because we interpret DMMs as not having channels), then that attribute won't even be accessible as a Python attribute in the all_channels object. If all attributes fall under the same pattern, then the all_channels will simply be an object with no Python attributes. Thus, users won't be able to use it for NI-DMM, but it will still be meaningful for other drivers. I like this because it feels very pure without losing practicality. |
Functions are easy since they either have a channel string parameter or not. And, this cannot change over time since that would break code. If the function started off without a channel string and then the driver team decided they needed one, they would have to create a completely new function. We do have information about whether a given attribute is channel based or not. However, when @marcoskirsch and I were discussing whether to use that information, we decided not to because:
It does solve how to deal with NI-DMM's lack of channels though while allowing the others to be consistent with NI-DAQmx and NI-XNET. All of the NI-DMM attribute have |
Taking into account @MrElusive 's comment, and the things that @texasaggie97 mentioned about our C APIs having the ability to "promote" a device attribute to a channel attribute or even not be consistent across models within a single API, I think the decision is sort of made for us... leave things as is. There's little benefit to introducing the ability to do |
I am closing this as |
Once #718 is merged, consider our API:
Issue 1
Does it matter?
Issue 2
Another potentially confusing problem is that all attribute are accessed on the
Session
but also by indexing thechannels
container:Does it matter?
Issue 3
Functions that are channel based are similar to attributes. But those that aren't (i.e.
commit()
) are only available on theSession
, not thechannels
container., except that functions that are _notDoes it matter?
Issue 4
Some suggested to follow nidaqmx-python precedent and have an "all_channels" member.
But then what about
nidmm
which has one channel (some would say no channels) and most of the C/LV API doesn't even have channel inputs? This is hardly ideal:The text was updated successfully, but these errors were encountered: