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

Missing usage of slots in classes with DAQmx attributes #656

Closed
zhindes opened this issue Nov 7, 2024 · 1 comment · Fixed by #664
Closed

Missing usage of slots in classes with DAQmx attributes #656

zhindes opened this issue Nov 7, 2024 · 1 comment · Fixed by #664

Comments

@zhindes
Copy link
Collaborator

zhindes commented Nov 7, 2024

A few classes that contain DAQmx attributes are missing slots declarations. This can cause confusion if a user typo's one of our attribute names and then nothing useful happens.

For example, this code doesn't do what you expect, because the attribute is called samp_quant_samp_per_chan even though the cfg_implicit_timing function variable name is samps_per_chan for the same setting. Fun!

with nidaqmx.Task() as co_task:
    co_task.co_channels.add_co_pulse_chan_freq("Dev1/ctr0", freq=10000, duty_cycle=0.5)
    co_task.co_channels.all.co_pulse_term = ""
    co_task.timing.cfg_implicit_timing(sample_mode=nidaqmx.constants.AcquisitionType.FINITE, samps_per_chan=256)

    for i in range(0, 100):
        number_of_samples = random.randint(2, 256)

        # Update the CO task
        co_task.timing.samps_per_chan = number_of_samples # <-- THIS IS DOING NOTHING!
        print(f"Number of samples: {co_task.timing.samps_per_chan}")
        co_task.start()
        co_task.wait_until_done()
        co_task.stop()

Templates that have attributes that don't have slots (bad!):

There are some iffy ones... e.g., collections don't have properties, only functions. But maybe they should have slots not to confuse people? idk. https://github.com/ni/nidaqmx-python/blob/master/src/codegen/templates/task/collections/_ai_channel_collection.py.mako. I think starting with that list above is good enough for now.

Note: this was create in response to an internal issue: Bug 2924340: Setting non-existent attribute on the CO timing does not cause assertion in python

@bkeryan
Copy link
Collaborator

bkeryan commented Nov 7, 2024

FYI, https://stackoverflow.com/questions/472000/usage-of-slots has some interesting details:

  • You can still allow creating new fields if you add __dict__ to slots. We would want to avoid this.
  • Similarly, slots disables weakref support unless you add __weakref__ to slots. It may make sense to continue to allow weak references to Task objects, if nothing else.
  • Setting slots may interfere with assigning __class__ unless the slots match. I don't think this is a problem because we only do this with subclasses and all of the subclasses set slots already.

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

Successfully merging a pull request may close this issue.

2 participants