-
Notifications
You must be signed in to change notification settings - Fork 118
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
Added dict key validation & updated documentation #417
Conversation
Codecov Report
@@ Coverage Diff @@
## main #417 +/- ##
=======================================
Coverage 94.50% 94.51%
=======================================
Files 103 104 +1
Lines 6448 6469 +21
=======================================
+ Hits 6094 6114 +20
- Misses 354 355 +1
📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
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.
Thanks for fixing these!
Should there be some sort of key validation then? To ensure we aren't passing in dict entries that are silently inactive? |
That's true, I'll add it. |
Added key validation as @nwu63 suggested, ready for the 2nd review |
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.
Thanks for doing this tedious work. Only one very minor spelling correction, and otherwise looks good!
User-defined surface dict | ||
""" | ||
|
||
# NOTE: make sure this is consistend to the documentation's surface dict page |
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.
Consistend -> consistent
Thanks for the quick review! |
Purpose
Key validation
With this PR, OAS will check a user-defined dicts (mesh dict or surface dict) and warn if the user provides a key that is not implemented or used in OAS.
For example, some examples and tests provide the
chord
key when creating a mesh, but the keychord
does not exist and therefore never used. I fixed them toroot_chord
, the one actually used.I also removed unused keys from other tests.
Documentation update
The current documentation and docstrings say the
chord_cp
variable is the actual chord value in meters, but what the code (ScaleX component) actually does is that it applies the chord scaler to the initial mesh. Thereforechord_cp
is actually a dimensionless chord scaler.This PR fixes the documentation and docstrings.
Also, I updated the surface dict documentation.
(It's a little too late but I probably should've opened two separate PRs for this...)
Expected time until merged
Type of change
Testing
I added a new test to check the chord values before and after
ScaleX
is applied.Checklist
flake8
andblack
to make sure the Python code adheres to PEP-8 and is consistently formattedfprettify
or C/C++ code withclang-format
as applicable