docs: Clarify test comment about ValueError from resolve_entity validation #399
Conversation
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
5 tasks
…ation Co-authored-by: Chaffelson <[email protected]>
Copilot
AI
changed the title
[WIP] Update scheduling and magic entity resolution feature
docs: Clarify test comment about ValueError from resolve_entity validation
Jan 6, 2026
Chaffelson
merged commit 51339ee
into
feature/port-scheduling-and-magic-strings
1 check passed
Chaffelson
added a commit
that referenced
this pull request
Jan 7, 2026
* Add get_port and schedule_port functions
- Add get_port(identifier, identifier_type) for retrieving ports by ID or name
- Add schedule_port(port, scheduled) following schedule_processor pattern
- Register PortEntity in config.py for filter_obj support
- Add comprehensive tests for both functions
* fix(canvas): Set controller name in initial creation request
Previously create_controller() set the name in a separate update call
after creation, but returned the stale object from the initial call.
This caused component.name to show the type name instead of the custom
name.
Now sets name directly in the ControllerServiceDTO during creation,
matching the pattern used by create_processor(). Also defaults to
short type name if no name provided (e.g., 'JsonTreeReader').
* WIP: Add scheduling state consolidation utilities
Utilities implemented in nipyapi/utils.py:
- resolve_schedule_state(): Standardized bool/string to state normalization
- resolve_entity(): Entity resolution accepting ID, name, or object
(to be used in next phase)
Scheduling functions consolidated in canvas.py:
- schedule_processor: Uses resolve_schedule_state()
- schedule_port: Uses resolve_schedule_state()
- schedule_controller: Uses resolve_schedule_state(), now accepts string
states ('ENABLED', 'DISABLED') in addition to bool
- schedule_all_controllers: Same extension
- schedule_components: Now accepts string states ('RUNNING', 'STOPPED')
Breaking change: Invalid scheduled values now raise ValueError instead
of AssertionError. This is a more appropriate exception type.
Tests expanded for new string state acceptance in controller scheduling.
All 660 tests pass.
* feat: Add resolve_entity utility for consistent entity resolution across modules
- Add resolve_entity() and resolve_schedule_state() utilities in utils.py
- Refactor 17 canvas.py functions to accept object, ID, or name
- Refactor 9 versioning.py functions with consistent entity resolution
- Refactor 5 parameters.py functions with consistent entity resolution
- Add schedule_port() and get_port() functions to canvas.py
- Add greedy parameter to get_controller()
- Remove deprecated get_variable_registry/update_variable_registry
- Consolidate verification request pattern with _run_verification_request helper
- Make location optional in create_process_group and create_funnel
- Add PortEntity to registered_filters in config.py
- Optimize fix_state_flow fixture to module scope
- Update tests for new ValueError exceptions and expanded functionality
Closes #396
* Add Raises sections to entity resolution function docstrings
* docs: Clarify test comment about ValueError from resolve_entity validation (#399)
* Initial plan
* docs: Clarify test comment about ValueError from resolve_entity validation
Co-authored-by: Chaffelson <[email protected]>
---------
Co-authored-by: copilot-swe-agent[bot] <[email protected]>
Co-authored-by: Chaffelson <[email protected]>
---------
Co-authored-by: Copilot <[email protected]>
Co-authored-by: Chaffelson <[email protected]>
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.
Addresses feedback on PR #398 requesting more precise test comment about exception type change.
Changes
test_revert_flow_verto explain exception type changed fromAssertionErrortoValueErrorbecauseresolve_entity()validates entity existence and raisesValueErrorfor validation errors, whereas previousisinstancecheck raisedAssertionErrorThe original comment stated "now raises ValueError (not found) instead of AssertionError" without explaining why. The updated comment clarifies the underlying cause: the refactoring to use
resolve_entity()for consistent entity resolution across modules changed the validation mechanism and its exception type.✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.