Expose NI-DCPower Advanced Sequences in Python API

[marcoskirsch]

The way the C API works, the client is supposed to pass an array of attribute IDs that he/she wants included in the advanced sequence. In our Python API, right now, this is just an int :(

This is not viable to expose as-is in Python because in the Python API clients never deal with attribute IDs.

So how to expose? Below some options, all viable but with no implementation details described.

Option 0: Enum for all attributes

We create an enum that contains all the attributes. The corresponding values are the attribute IDs.

Pros

• Very similar to LabVIEW API

Cons

• Introduces an enum
• ... which uses different conventions that the Python attribute names

session.source_mode = nidcpower.SourceMode.SEQUENCE
attribute_ids = [
    nidcpower.Attributes.OUTPUT_FUNCTION,
    nidcpower.Attributes.VOLTAGE_LEVEL,
    nidcpower.Attributes.CURRENT_LEVEL
]
session._create_advanced_sequence(sequence_name='my_sequence', attribute_ids=attribute_ids)

session._create_advanced_sequence_step()
session.output_function = nidcpower.OutputFunction.DC_VOLTAGE
session.voltage_level = 5.0

session._create_advanced_sequence_step()
session.output_function = nidcpower.OutputFunction.DC_CURRENT
session.current_level = 0.1

Option 1: List of dicts for the whole sequence

Whole sequence is specified as a list of dicts. Each dict represents a step. Each key in a dict is a string that must match the Python attribute name.

session.source_delay = datetime.timedelta(seconds=0.1)
session.voltage_level_autorange = True
session.current_level_autorange = True

session.source_mode = nidcpower.SourceMode.SEQUENCE
my_advanced_sequence = [
    {"output_function":nidcpower.OutputFunction.DC_VOLTAGE, "voltage_level":5.0},
    {"output_function":nidcpower.OutputFunction.DC_CURRENT, "current_level":0.1}
]
session.create_advanced_sequence(sequence_name='my_sequence', my_advanced_sequence)

Pros

• Very Pythonic
• Very compact

Cons

• Different than C and LabVIEW paradigm
• While we can expose it, I'd rather not expose modifying steps

Option 2: Use list of names to specify attributes in an active sequence

Pros

• No need to create a new enum
• Attribute names specified when creating the sequence use Python convention / match Session
• Better matches C / LabVIEW APIs

Cons

• Not Pythonic

Option 3: Sequence object

A new type AdvancedSequence is introduced into the API. The user can create one of these and it acts as a list (iterable, etc). The user adds / modifies steps at will then passes it to nidcpower.Session.create_advanced_sequence(name, sequence). Presumably, at this point we create it, but allow further modifications.

Pros

• Very flexible
• Pythonic

Cons

• Different than C and LabVIEW paradigm
• Seems hard to use / reason about (for debate)

[marcoskirsch]

I believe that if we cannot fix this on time for the 0.9 release, the feature is fairly unusable and we may be better off turning off code-gen for all the Advanced Sequence related functions.

[marcoskirsch]

We decided to make all the advanced sequence functions private until we can come up with a suitable, Pythonic, solution.
What would that be, I'm not sure.

[marcoskirsch]

Proposed "Pythonic" advanced sequence API.

with nidcpower.Session(resource_name="Dev1", channels=[0,1,2]) as session:

    session.source_delay = datetime.timedelta(seconds=0.1)
    session.voltage_level_autorange = True
    session.current_level_autorange = True

    # This is how you create an advanced sequence.
    session.source_mode = nidcpower.SourceMode.SEQUENCE
    my_advanced_sequence = [
        {"output_function"=nidcpower.OutputFunction.DC_VOLTAGE, "voltage_level"=1.0},
        {"output_function"=nidcpower.OutputFunction.DC_CURRENT, "current_level"=0.1}
    ]
    session.create_advanced_sequence(sequence_name='my_sequence', my_advanced_sequence)
    # Neat, right?

[claytonlemons]

Looks very neat!

Pros

• All advanced sequencing attributes are specified upfront, so create_advanced_sequence can scan the dict to get the attribute names (and thereby the attribute ids).
• It's clear what attribute values each step contains.
• It'd be very easy to map some JSON data to an advanced sequence.

Caveats

• This may necessitate that we hide niDCPower_CreateAdvancedSequenceStep from the API. The reason is because it may seem strange and misleading to mix calls to that entry point along with the proposed approach.

    session.create_advanced_sequence(sequence_name='my_sequence', my_advanced_sequence)

    session.create_advanced_sequence_step()
    session.output_function = nidcpower.OutputFunction.DC_VOLTAGE
    session.source_delay = datetime.timedelta(seconds=0.5)
    # If the user was expecting to change source_delay for only this new step,
    # it will not have the intended effect, nor will it error. This is a problem
    # even for the LV API, but at least we have the flexibility to protect
    # against it by hiding create_advanced_sequence_step

• Modifying steps would also have a similar problem:

    session.create_advanced_sequence(sequence_name='my_sequence', my_advanced_sequence)

    session.active_advanced_sequence_step = 0
    session.output_function = nidcpower.OutputFunction.DC_CURRENT
    session.source_delay = datetime.timedelta(seconds=0.5)  
    # Changes source delay for all steps, not just the currently active one!
    # However, I don't think we can get away with hiding the 
    # active_advanced_sequence_step attribute.

Other Thoughts

• If you are able to map attribute names to attribute ids, you could just pass a list of attribute names rather than a dict to create_advanced_sequence. The user could then configure each step using just the session.

[marcoskirsch]

This may necessitate that we hide niDCPower_CreateAdvancedSequenceStep from the API. The reason is because it may seem strange and misleading to mix calls to that entry point along with the proposed approach.

Definitely. There would be no public nidcpower.Session.create_advanced_sequence_step().

Modifying steps would also have a similar problem

Agreed. I propose not allowing it from the Python API. Is this is too restrictive, we can look into exposing it somehow.

If you are able to map attribute names to attribute ids, you could just pass a list of attribute names rather than a dict to create_advanced_sequence. The user could then configure each step using just the session.

Yeah, we could do this. We'd use the Python names for the attributes as well. This would be closer to the C and LabVIEW APIs. Let's refer to this as Option 2.

[thejcannon]

I don't think I could've done it any better myself. Looks very natural!

[thejcannon]

After you get the prototype working, I'd try to make sure it plays nice with other iterable types that satisfy all the requirements (like a generator perhaps?).
The only caveat I see would be that you probably iterate over the "sequence" twice (once to get the IDs and then once for the step creation), which generators don't allow. But that can be remedied in a handful of ways.
Not necessary for v1 of the feature, but food for thought.

[marcoskirsch]

it plays nice with other iterable types

What's "it" in this context?

[thejcannon]

Additionally, how does this effect attributes like active_advanced_sequence and active_advanced_sequence_step?
There's an Option 3 out there where when the user selects source_mode = SEQUENCE, allows them to modify some object associated with the session that is the abstraction of the Advanced Sequence(s). The object would act like a list (in that steps could be added, but also in that it can be indexed to be modified). You (the API author) could decide if the Advance sequence object holds onto the information and then interacts with the API when the user tells you, or if the object operates in terms of DC-Power calls, using active_advanced_sequence and active_advanced_sequence_step.

[thejcannon]

it plays nice with other iterable types

What's "it" in this context?

The second argument given to create_advanced_sequence()

[marcoskirsch]

Additionally, how does this effect attributes like active_advanced_sequence and active_advanced_sequence_step?

I don't think it impacts active_advanced_sequence, and I lean towards not exposing active_advanced_sequence_step.

There's an Option 3 out there where when the user selects source_mode = SEQUENCE, allows them to modify some object associated with the session that is the abstraction of the Advanced Sequence(s). The object would act like a list (in that steps could be added, but also in that it can be indexed to be modified).

It's an interesting idea worth considering. We have 3 now, so I'll try to capture in the original issue description.

[marcoskirsch]

So far, I like options 1 and 2 in that order. Option 0 is nasty and option 3 seems pretty complicated

[epage]

Could someone clarify what is wrong with Option 0. Generally the goal is to not have stringly typed APIs. They are easy to mess up and hard to know what is accepted.

EDIT: I guess I'm focusing more on the "enum" part of Option 0 and not how the API is sequenced / the data structure

[marcoskirsch]

I dislike that the client refers to a property (e.g. Output Function) in two ways throughout the program. In the normal case: my_session.output_function. But when using it in a sequence: nidcpower.Attributes.OUTPUT_FUNCTION.

@epage would you say you favor Option 1, but want the keys (properties) to be in the from instead of referring to properties in the form nidcpower.Attributes.OUTPUT_FUNCTION rather than in the form "output_function"?

[epage]

@epage would you say you favor Option 1

I'm still trying to come to understand the API and the usability issues associated with any proposal.

So from talking with Sydney, it sounds like

• the user is left in the last step of the last sequence for editing.
• like with any properties, there is coercion going on and being able to introspect on the steps can be useful

From, this, I'm concerned about an API that offers a "one-shot" approach for creating a sequence. The user will expect the sequence to be created and it will be surprising that the session is left at the last step of the last sequence for any further operations they do. This violates the principle of least surprise.

I don't think it impacts active_advanced_sequence, and I lean towards not exposing active_advanced_sequence_step.

Why is active_advanced_sequence_step not needed? You gave your opinion but didn't say why?

It seems like it'd be useful for checking how coercion happened, reconfiguring the sequence, etc.

[epage]

Crazy idea

sequence = session.create_sequence(name, attrs)
sequence.create_step()
session.output_enabled = True
sequence.create_step()
session.output_enabled = False
sequence.create_step()
session.output_enabled = True
sequence.step = 2

output_enableds = [false, true., false]
for output_enabled, step in zip(session.create_sequence(name, attrs), outpout_enableds):
  session.output_enabled = output_enabled


# Why not have attribute get/sets on `sequence`?
# No indication of interplay with session, particularly that gets/sets on session are related to the step

[epage]

Another idea that has the same problem as Option 1

1. Pass a sequence object to create_advanced_sequence

It records your actions and then the call replays it

2. create_advanced_sequence returns a proxy object with attributes that record. The close / context manager woudl then apply them.

[sydneypjohnson]

@epage To add a few more details that I left out in our call. This comment is based on the current API:

the user is left in the last step of the last sequence for editing.

There's a couple different states (regarding the advanced sequence and the properties) the user could enter after calling create_advanced_sequence in the current API:

1. As you mentioned, they're left in the last step of the newly created sequence. Accessing any properties that were provided in the sequence parameter will access the value for the last step. If advanced_sequence_step is exposed, they could change to any step they want to get/set properties.
2. The user could set active_advanced_sequence back to "", which turns off advanced sequencing. Accessing any properties that were provided in the sequence parameter will access the "global" value (the one that isn't associated with any advanced sequence). The user will also have to set source_mode back to single point.
3. The user could switch to another (previously created) advanced sequence by setting active_advanced_sequence. However, if advanced_sequence_step isn't exposed, this might not work. I don't think NI-DCPower automatically sets advanced_sequence_step to a valid value when the user changes active_advanced_sequence. So if the user creates a sequence A with 3 steps, then a sequence B with 4 steps, then tries going back to sequence A, they will probably get an error (since A doesn't have 4 steps).

like with any properties, there is coercion going on and being able to introspect on the steps can be useful

There could also be dependencies between properties that were specified in the sequence parameter and properties that weren't. For instance, if the user sets voltage_level_autorange to On and calls create_advanced_sequence with voltage_level specified in the sequence, the user may want to read the voltage_level_range property to see which range was selected.

[epage]

The user could set active_advanced_sequence back to "", which turns off advanced sequencing. Accessing any properties that were provided in the sequence parameter will access the "global" value (the one that isn't associated with any advanced sequence). The user will also have to set source_mode back to single point.

This reminds me of something else. It is unfortunate that active_advanced_sequence changes sequence being edited and ran while active_advanced_sequence_step only changes what step is being edited.

imo We should try to paper over this in the API. For example, see my Crazy Idea.

If we had a way to edit a sequence without changing the active one, we could revert back to editing "" and all would be dandy. This would be my ideal API.

[sydneypjohnson]

we could revert back to editing "" and all would be dandy. This would be my ideal API.

Do you mean that the active_advanced_sequence would be "" after calling create_advanced_sequence()? Keep in mind that the active_advanced_sequence must be set to the user's desired sequence by the time they call initiate() so that the device executes it. Also keep in mind that setting active_advanced_sequence in the driver could cause the session to transition from the Committed -> Idle state (like setting any other attribute/property). So active_advanced_sequence must be set appropriately before the user calls commit() / initiate().

I do agree, though, that it might be confusing to the user that create_advanced_sequence() leaves the active step set to the last one. This is a bit more transparent in the C API since there the user must create each step themselves.

[epage]

@sydneypjohnson

Do you mean that the active_advanced_sequence would be "" after calling create_advanced_sequence()? Keep in mind that the active_advanced_sequence must be set to the user's desired sequence by the time they call initiate() so that the device executes it

The key phrase in my statement "If we had a way to edit a sequence without changing the active one,"

[texasaggie97-zz]

Did we ever make a decision on how to implement this? So far, the PR is using string based names and is not supporting editing/modifying of the sequence after the fact.

[marcoskirsch]

"Option 1" doesn't support multi-channel sequences. NI-DCPower doesn't support multi-channel sequences today, but the API (C, LabVIEW) can be trivially expanded to do so, and there is good reason to assume NI-DCPower will add multi-channel sequences in the future.

How can "Option 1" be expanded to support multi-channel sequences?

[marcoskirsch]

Met with @texasaggie97, Tom the NI guy that knows NI-DCPower the best, @thejcannon to discuss this.

From Zen of Python

Special cases aren't special enough to break the rules.

There should be one-- and preferably only one --obvious way to do it.

We decided to expose this functionality in a way that maps directly to the C and LabVIEW APIs, except we will use a list of strings with the Python names of the properties in create_advanced_sequence instead of an enum or attribute IDs which don't exist to the Python user.