-
Notifications
You must be signed in to change notification settings - Fork 0
First pass of minimal Python API #8
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR introduces a minimal Python API for the NI Data Store Service with stub implementations to establish the basic interface design. The changes focus on creating a foundation for further development and testing of the API.
- Adds a
Clientclass with methods for publishing/reading measurement data and creating test steps/results - Includes commented test code demonstrating expected API usage patterns
- Updates project dependencies to support the new datastore functionality
Reviewed Changes
Copilot reviewed 5 out of 7 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ni/datastore/client.py | New client implementation with stub methods for datastore operations |
| src/ni/datastore/init.py | Exports the Client class for public API access |
| tests/unit/test_ni_datastore_client.py | Adds commented test showing expected API usage |
| pyproject.toml | Adds required dependencies for datastore functionality |
| docs/conf.py | Updates documentation configuration with warning suppressions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| moniker.service_location = "localhost:50051" | ||
| client.read_measurement_data(moniker, Vector) | ||
| mocked_moniker_client.read_from_moniker.return_value = ReadFromMonikerResult() | ||
| client.read_measurement_data(moniker, Any) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How did this pass? typing.Any is only for type checking, not for run time. Use object if you want to short-circuit the type check.
>>> import typing
>>> isinstance(3, typing.Any)
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
File "C:\Users\bkeryan\.pyenv\pyenv-win\versions\3.11.9\Lib\typing.py", line 515, in __instancecheck__
raise TypeError("typing.Any cannot be used with isinstance()")
TypeError: typing.Any cannot be used with isinstance()There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Tests should use triple underscore as a separator.
What does this Pull Request accomplish?
Add some stubs for the Python API so we can review it and hash out the types we'll be using and the basic form of the API. Once we agree on some basic stuff, we can work on implementation and testing.
Why should this Pull Request be merged?
Simple start to get the API rolling.
What testing has been done?
Add a (commented out) test that shows what the API would look like from the user's perspective.