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

add slots to DAQmx-property-containing classes #664

Merged
merged 8 commits into from
Dec 17, 2024
Merged

Conversation

zhindes
Copy link
Collaborator

@zhindes zhindes commented Dec 13, 2024

  • This contribution adheres to CONTRIBUTING.md.
  • I've updated CHANGELOG.md if applicable.
  • I've added tests applicable for this pull request

What does this Pull Request accomplish?

Prevents inadvertent misuse of the DAQmx API by raising an AttributeError if a user attempts to use a DAQmx Property that doesn't exist. Closes #656.

Why should this Pull Request be merged?

This avoids really tricky bugs.

What testing has been done?

  • Added tests for all the modified attribute-containing classes.
  • Added tests for weakref support on Task and WatchdogTask, since we want to maintain that.
  • Ran the rest of existing tests.

Copy link

github-actions bot commented Dec 13, 2024

Test Results

0 files   -     40  0 suites   - 40   0s ⏱️ - 1h 2m 33s
0 tests  -  2 350  0 ✅  -  1 996  0 💤  -   354  0 ❌ ±0 
0 runs   - 49 828  0 ✅  - 42 484  0 💤  - 7 344  0 ❌ ±0 

Results for commit 8ebd5f8. ± Comparison against base commit d2e7efc.

♻️ This comment has been updated with latest results.

Copy link
Collaborator

@bkeryan bkeryan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Approved with suggestions

tests/component/system/test_system.py Outdated Show resolved Hide resolved
src/handwritten/task/_task.py Show resolved Hide resolved
tests/component/test_watchdog.py Outdated Show resolved Hide resolved
@zhindes zhindes enabled auto-merge (squash) December 17, 2024 20:44
@zhindes zhindes disabled auto-merge December 17, 2024 21:11
@zhindes zhindes enabled auto-merge (squash) December 17, 2024 21:11
@zhindes zhindes merged commit 6b14958 into master Dec 17, 2024
15 checks passed
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 this pull request may close these issues.

Missing usage of slots in classes with DAQmx attributes
2 participants