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

Make user-visible types public to support type hints #461

Open
bkeryan opened this issue Dec 11, 2023 · 5 comments
Open

Make user-visible types public to support type hints #461

bkeryan opened this issue Dec 11, 2023 · 5 comments
Assignees

Comments

@bkeryan
Copy link
Collaborator

bkeryan commented Dec 11, 2023

The task subobject types (AIChannel, Timing, etc.) are defined in internal modules contained in the private nidaqmx._task_modules package. When users write helper functions that accept channel objects or other task subobjects, they must refer to these internal modules. We should make these modules public and/or add public aliases for their types.

@StSav012 brought this up in #458

@bkeryan
Copy link
Collaborator Author

bkeryan commented Dec 11, 2023

I think we should turn task.py into a package, merge it with _task_modules, make most of the submodules private, and add aliases to reexport types from the subpackages.

Something like this:

nidaqmx/
  task/
    __init__.py - defines Task; reexports ExportSignals, InStream, OutStream, Timing, Triggers
    channels/
      __init__.py - reexports Channel, AIChannel, AOChannel, etc.
      _channel.py - defines Channel
      _ai_channel.py - defines AIChannel
      ...
    collections
      __init__.py - reexports ChannelCollection, AIChannelCollection, AOChannelCollection, etc.
      _channel_collection.py - defines ChannelCollection
      _ai_channel_collection.py - defines AIChannelCollection
      ...
    _export_signals.py - defines ExportSignals
    _in_stream.py - defines InStream
    _out_stream.py - defines OutStream
    _timing.py - defines Timing
    _triggers.py - defines Triggers
    triggering/
      __init_.py - reexports triggering classes
      _arm_start_trigger.py - defines ArmStartTrigger
      ...

The public types would be:

nidaqmx.task.Task
nidaqmx.task.ExportSignals
nidaqmx.task.InStream
nidaqmx.task.OutStream
nidaqmx.task.Timing
nidaqmx.task.Triggers
nidaqmx.task.channels.Channel, AIChannel, etc.
nidaqmx.task.collections.ChannelCollection, AIChannelCollection, etc.
nidaqmx.task.triggering.ArmStartTrigger, HandshakeTrigger, etc.

@charitylxy
Copy link
Collaborator

Completed refactoring work for task and its submodules, documentation is updated accordingly. Closing this issue.

@bkeryan
Copy link
Collaborator Author

bkeryan commented Mar 21, 2024

Some of the types in nidaqmx.system are not public yet. Also, the docs refer to nonexistent modules like nidaqmx.system.collections and nidaqmx.system.device_collection:
https://nidaqmx-python.readthedocs.io/en/latest/collections.html

(This is really nidaqmx.system._collections, which is private.)

@bkeryan bkeryan reopened this Mar 21, 2024
@charitylxy
Copy link
Collaborator

charitylxy commented Mar 22, 2024

@bkeryan The things needed to update are:

  1. nidaqmx.system.collections: making the submodules in collections private and update docs
  2. nidaqmx.system.storage: upadate docs
  3. nidaqmx.system.watchdog: making _watchdog_modules public and its submodules private and update docs

Anything else i miss out?

@bkeryan
Copy link
Collaborator Author

bkeryan commented Mar 22, 2024

nidaqmx.system.collections: making the submodules in collections private and update docs

Also make nidaqmx.system.collections itself public & add aliases for the collection types.

nidaqmx.system.storage: upadate docs

nidaqmx.system.storage and nidaqmx.system.storage.persisted_channel are both public, so the only thing that is entirely wrong here is the docs.

However, it's confusing that you can import PersistedChannel from either nidaqmx.system.storage and nidaqmx.system.storage.persisted_channel. That fails "There should be one-- and preferably only one --obvious way to do it." from the Zen of Python.

It would make sense to shorten nidaqmx.system.storage.persisted_channel.PersistedChannel -> nidaqmx.system.storage.PersistedChannel for consistency with the other subpackages like collections, channels, etc. and keep the old submodules around as deprecated compatibility shims.

nidaqmx/system/storage/
   __init__.py: Exports PersistedChannel, PersistedTask, PersistedScale
   _persisted_channel.py, etc.: Defines PersistedChannel, etc.
   persisted_channel.py, etc.: Announces a DeprecationWarning and re-exports PersistedChannel or whatever.

nidaqmx.system.watchdog: making _watchdog_modules public and its submodules private and update docs

I think you should turn watchdog into a package like you did with task.

nidaqmx/system/watchdog/
   __init__.py: Exports WatchdogTask, ExpirationState, ExpirationStateCollection
   _watchdog_task.py: Renamed version of watchdog.py
   _expiration_state.py
   _expiration_state_collection.py

(Or you can rename watchdog.py to watchdog/__init__.py if that's easier.)

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

No branches or pull requests

2 participants