standalone-tsm-context - get_context_with_sites#167
standalone-tsm-context - get_context_with_sites#167devin-roffman-emerson merged 26 commits intoni:mainfrom
Conversation
…ntext in kwargs Added get_semiconductor_module_context_with_sites to get context with specific sites. Modified codemoduleapi.py - __call__ to check for context in kwargs as a fallback
prithierajmr
changed the title
There was a problem hiding this comment.
Pull request overview
Adds APIs to better support standalone Semiconductor Module context usage by enabling site-filtered contexts and attempting to allow passing the context via keyword arguments to code modules.
Changes:
- Added
SemiconductorModuleContext.get_semiconductor_module_context_with_sites(...)wrapper around the COM APIGetSemiconductorModuleContextWithSites. - Updated
code_module.__call__to attempt a kwargs-based fallback for locating the TSM context and adjusted the related error message.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 7 comments.
| File | Description |
|---|---|
src/nitsm/tsmcontext.py |
Introduces a new public method to obtain a site-filtered SemiconductorModuleContext. |
src/nitsm/codemoduleapi.py |
Attempts to add kwargs fallback behavior for locating the context when invoking decorated code modules. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Changed the type to Sequence of integers for Py <3.7 compatibility Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove the print statement added for debugging Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Typo in the returns - actual return TSM Context is for the specified site_numbers Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…sm_context in kwargs tsmcontext - dumbdispatch for getting context with sites codemoduleapi - Typo Added codemoduleapi test for checking tsm_context position
…ecking tsm_context in kwargs" This reverts commit ffa316a. Rewriting the changes, since we do not want to use dumbdispatch and remove the changes for code_moduleapi
This reverts commit 51978c3.
This reverts commit 48cba71.
This reverts commit 2fff250.
This reverts commit b2e716b.
This reverts commit 3bec2bf.
This reverts commit be9f63a.
…hecking tsm_context in kwargs" This reverts commit 78cc0ce.
…ecking tsm_context in kwargs" This reverts commit ffa316a.
This reverts commit 51978c3.
This reverts commit 48cba71.
This reverts commit 2fff250.
…r tsm_context in kwargs" This reverts commit ef315a7.
…h the get-context-with-sites method and added a test under 'test_multi_site.py' file with the corresponding pinmap
…h the get-context-with-sites method and added a test under 'test_multi_site.py' file with the corresponding pinmap
prithierajmr
changed the title
prithierajmr
left a comment
prithierajmr
left a comment
There was a problem hiding this comment.
@ktvanzwol @devin-roffman-emerson - I reverted all the previous changes and just added get_semiconductor_module_with_sites method with the corresponding tests. Hope this change should be the right one. Please review and let me know
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Make sure to run the formatter, Black, to format the code in this PR so that it matches the expected TSM format. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove unused imports Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Increase the coverage of tests by calling other methods on the context Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Remove all_sites, since they are not used anyway Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
…in test_multi_site.py for validating the functionality
|
Closed all the review comments @devin-roffman-emerson , @ktvanzwol |
|
@prithierajmr The lint step is failing. Please rerun the Black formatter again. I am running the pipeline on our end and if all tests pass, we should be good to merge. |
devin-roffman-emerson
assigned
devin-roffman-emerson and unassigned
devin-roffman-emerson
|
@devin-roffman-emerson , I have reformatted using Black on src and tests directories. Should be good to go if there are no failures. |
Added get_semiconductor_module_context_with_sites to get context with specific sites. Modified codemoduleapi.py - call to check for context in kwargs as a fallback
TODO: Check the above box with an 'x' indicating you've read and followed CONTRIBUTING.md.
What does this Pull Request accomplish?
Why should this Pull Request be merged?
For utilizing the Standalone Semiconductor Module Context effectively, this function are required
What testing has been done?
Ran the automated tests completely to validate these functions and made sure these functions does not affect any other functionality