Add dict-style component access to Model#226
Closed
FBumann wants to merge 1 commit intoBravos-Power:mainfrom
Closed
Add dict-style component access to Model#226FBumann wants to merge 1 commit intoBravos-Power:mainfrom
FBumann wants to merge 1 commit intoBravos-Power:mainfrom
Conversation
- Added "_components" to _reserved_attributes
- Initialized self._components: dict[str, Any] = {} at the top of __init__
- Extracted _register_component(name, value) — the shared logic for type validation, duplicate check (now name in self._components),
_on_add_to_model, list/map updates, logging, and storing in _components
- Refactored __setattr__ to delegate to _register_component for non-reserved names
- Added __setitem__ with reserved-name guard, calls _register_component, and also sets as attribute if the name is a valid identifier
- Added __getitem__ looking up from _components
- Added __contains__ checking _components
src/pyoframe/_core.py
- Variable._on_add_to_model: replaced setattr(model, ...) with model[...] = ... for _lb, _ub, _equals derived constraints
- Constraint.relax: replaced hasattr(m, var_name) with var_name not in m, and setattr(m, ...) with m[...] = ...
tests/test_names.py
- Added 8 new tests covering: non-identifier names, unified access (attr↔dict), __contains__, expression bounds with non-identifier names, reserved
name error, duplicate error, invalid type error, missing key error, and end-to-end solve
Test results: 146 passed, 2 failed (pre-existing: ipopt/copt not installed), 105 skipped.
FBumann
changed the title
staadecker
added
the
safe to test
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #226 +/- ##
==========================================
+ Coverage 96.60% 96.71% +0.10%
==========================================
Files 10 10
Lines 1563 1582 +19
==========================================
+ Hits 1510 1530 +20
+ Misses 53 52 -1
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
|
I'm going to close this for now until the conversation in #227 is resolved at which point I'd be very happy to reopen this PR! Thank you again for the contribution. |
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
2 participants
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.
Summary
m["name"] = .../m["name"]dict-style access for model components, enabling arbitrary string names (e.g."Variable--2 xy","con: budget ≤ 10")_componentsdict so attribute-assigned components are also accessible viam["X"]and vice versa__contains__support ("X" in m)Changes
src/pyoframe/_model.py— Extract_register_component()as single chokepoint for component registration. Add__setitem__,__getitem__,__contains__. Refactor__setattr__to delegate to_register_component.src/pyoframe/_core.py— Replacesetattr(model, ...)/hasattr(m, ...)calls inVariable._on_add_to_modelandConstraint.relaxwith dict-style access (model[name] = .../name not in m).tests/test_names.py— 8 new tests: non-identifier names, unified attr↔dict access,__contains__, expression bounds with non-identifier names, error cases (reserved name, duplicate, invalid type, missing key), and end-to-end solve.Test plan
test_names.pytests pass (16/16)🤖 Generated with Claude Code