-
Notifications
You must be signed in to change notification settings - Fork 119
Minor improvements to addConGroup
#463
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 improves the addConGroup function documentation and adds a validation warning. It fixes docstring errors and clarifies parameter descriptions, while also adding a runtime warning for constraints that have neither upper nor lower bounds defined.
- Fixed docstring description to reference constraints instead of variables
- Updated parameter documentation to clarify optional nature and default behaviors
- Added warning validation for constraints without bounds
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #463 +/- ##
==========================================
+ Coverage 86.26% 86.27% +0.01%
==========================================
Files 24 24
Lines 3429 3432 +3
==========================================
+ Hits 2958 2961 +3
Misses 471 471 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
| specifies for what dvs have non-zero Jacobian values | ||
| for this set of constraints. The order is not important. | ||
| wrt : iterable (list, set, OrderedDict, array etc), optional | ||
| 'wrt' stands for 'With Respect To'. This specifies which dvs this constraint is assumed to depend on, and |
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.
This can be expanded upon by explaining that it needs to be an iterable of str which specifies the DVGroups which are sensitive to.
Purpose
Was reading the docstring of
addConGroupand found a couple of mistakes and missing pieces of information so I fixed them. I also added a warning that will be raised when a constraint has neither upper nor lower bounds defined.Expected time until merged
no rush
Type of change
Testing
Checklist
ruff checkandruff formatto make sure the Python code adheres to PEP-8 and is consistently formattedfprettifyor C/C++ code withclang-formatas applicable