Update Inputs for User-Facing Classes #85
Conversation
|
I have another commit coming soon that will improve the use of |
|
PR is ready for review now! |
There was a problem hiding this comment.
This is a bigger upgrade than expected - thanks for all the effort @connoramoreno
Examples/example_python.py
Outdated
| num_s = 4 | ||
| num_theta = 8 | ||
| num_phi = 4 |
There was a problem hiding this comment.
While we're refining this, I've wondered about making this a tuple mesh_size = (4, 8, 4).
Pros:
- It makes things a little more compact
- It is a common pattern for sizing a grid
Cons:
- the order of (s, theta, phi) is implied
There was a problem hiding this comment.
I think with sufficient documentation, a tuple input would be fine
parastell/utils.py
Outdated
| for name, value in dict.items(): | ||
| if name in allowed_kwargs: | ||
| kwarg_dict.update({name: value}) | ||
| elif all_kwargs and name not in allowed_kwargs: |
There was a problem hiding this comment.
Isn't the second half of this condition implied by the fact that you didn't match on the if?
There was a problem hiding this comment.
Well I want the first condition to happen whether all_kwargs is True or False, while I want the error to be raised only under the condition that all_kwargs = True.
This functionality is particularly useful when extracting keyword arguments from YAML-derived dictionaries, since not all keys in those dictionaries are keyword arguments. In such cases, we set all_kwargs = False since we don't want an error to be raised when the algorithm comes across a key that is a positional argument.
There was a problem hiding this comment.
But I think you only get to this check against all_kwargs if name not in allowed_kwargs... right?
| elif all_kwargs and name not in allowed_kwargs: | |
| elif all_kwargs: |
There was a problem hiding this comment.
Oh, I see what you're saying. Good point! I thought the "second half of the condition" you were referring to was the elif as a whole.
connoramoreno
force-pushed
the
setters-getters
branch
from
c66c7b2 to
28f81cc
Compare
Reduces arguments of all user-facing classes to a minimal list and introduces setters and getters for those classes. Additionally, implements the use of
**kwargsto handle optional setting of default class attributes for both the Python API and command-line interface with YAML input.Furthermore, introduces a
RadialBuildclass ininvessel_build.pyto minimize the argument list forInVesselBuild.