Add default value to nislsc python api - #67
Open
zoechanzy wants to merge 6 commits into
Open
Conversation
--------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>
* Update function_helpers.py and templates files to consume default value field from nislscapi_full.json * Refactor the order of parameters to avoid python TypeError: positional argument follows keyword argument error * Regenerate generated Python module files --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
zoechanzy
force-pushed
the
users/zchan/nislsc_default_value
branch
3 times, most recently
from
August 6, 2026 05:54
af3371e to
6bb49f5
Compare
* Added default value of None for the library parameter * Regenerate generated Python module files --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>
zoechanzy
force-pushed
the
users/zchan/nislsc_default_value
branch
from
August 6, 2026 06:11
6bb49f5 to
646ecac
Compare
zoechanzy
changed the title
* Refactor parameter ordering in unit tests --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>
* Refactor parameter ordering in examples to match new defaults --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>
zoechanzy
force-pushed
the
users/zchan/nislsc_default_value
branch
from
August 7, 2026 03:12
851de41 to
fdd3ed9
Compare
There was a problem hiding this comment.
Pull request overview
This PR updates the codegen metadata and templates so generated NI-SLSC Python APIs can expose parameter default values (notably library=None, timeouts, and “default resource” sentinels), and regenerates the affected modules and tests accordingly.
Changes:
- Added
defaultfields tonislscapi_full.jsonfor various parameters (timeouts, default resources). - Updated codegen helpers/templates to emit defaults in generated Python signatures (and to reorder required vs defaulted parameters).
- Regenerated
generated/nislsc/session.pyand updated unit tests/examples to match the new call signatures.
Reviewed changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/unit/test_session.py | Updates test calls to match reordered/generated Session method signatures and new keyword usage. |
| tests/unit/conftest.py | Updates the session fixture to use new initialize_session_with_devices signature/keywords. |
| src/codegen/utilities/function_helpers.py | Adds “include defaults” support, default formatting helpers, and signature reordering logic. |
| src/codegen/templates/utils.py.mako | Enables default emission for module-level generated functions. |
| src/codegen/templates/session.py.mako | Enables default emission for generated Session methods/classmethods. |
| src/codegen/templates/property.py.mako | Enables default emission for generated PropertyReference APIs. |
| src/codegen/templates/library.py.mako | Enables default emission for generated Library APIs. |
| src/codegen/templates/command.py.mako | Enables default emission for generated CommandReference APIs. |
| src/codegen/metadata/nislscapi_full.json | Adds default values in metadata (timeouts, $Default* resource sentinels). |
| generated/nislsc/session.py | Regenerated Session API with defaults and reordered parameters. |
| examples/show_command_and_property_tree.py | Updates example calls to match new Session method parameter order. |
| examples/reset_device.py | Updates example initialization call to use new keyword/default patterns. |
| examples/check_chassis_battery.py | Updates example property calls to match new parameter ordering/defaults. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…types --------- Signed-off-by: zoechanzy <zoe.chan@emerson.com>
zoechanzy
marked this pull request as ready for review
August 7, 2026 05:36
tjying95
reviewed
Aug 10, 2026
| ) | ||
| physical_channel_properties = session.get_physical_channel_property_string_array( | ||
| physical_channel, PhysicalChannelProperty.PROPERTIES | ||
| PhysicalChannelProperty.PROPERTIES, physical_channel |
Collaborator
There was a problem hiding this comment.
@bkeryan With $DefaultPhysChans and $DefaultDevices set, we have to put the physical_channel and device_name to the back. Feels a bit weird (I am still fine with it).
Alternative is having positional arguments default to None or "", then we error out if not set. But that will probably mess the docstring and IntelliSense. Thoughts?
tjying95
reviewed
Aug 10, 2026
Comment on lines
52
to
55
| connection_timeout = 10.0 | ||
| reservation_access = ReservationAccess.READ_ONLY | ||
| reservation_group = "admin" | ||
| reservation_timeout = 10.0 |
Collaborator
There was a problem hiding this comment.
We now have defaults for connection_timeout and reservation_timeout. Let's just use them. This applies to other example too.
tjying95
reviewed
Aug 10, 2026
| reservation_access, | ||
| reservation_group, | ||
| reservation_timeout, | ||
| library=None, |
Collaborator
There was a problem hiding this comment.
Same for library. We now default this too.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What does this Pull Request accomplish?
Why should this Pull Request be merged?
nislscapi.json is updated to have default values. Function helper and template files should be updated so that the default value can be used in code-gen.
What testing has been done?
Passed unit tests