Streamline kwargs

[gonuke]

As I started looking at implementing NWL in the OO version, I realized that we were kind of heavy-handed with kwargs. I think this finds a better balance. Let me know what you think.

Some explanations for the changes:

1. The old code had the lists of allowable arguments defined in multiple places. They should be defined in as few places as possible - ideally only one.
2. Most of the uses of construct_kwargs_from_dict were for passing into a constructor for some class. That class is the best place to know what arguments are allowable, so its best to pass in the whole dictionary and then cherry-pick relevant data in the constructor.
3. set_kwarg_attrs was the function that cherry-picked from the dictionary, but raised exceptions by default for extra arguments. We probably discussed this at the time, and I may have recommended or endorsed it, but upon reflection it seems heavy-handed. It is only because of this that we had to carefully curate the kwargs that we passed in the first place and maybe its fine to just silently ignore extra kwargs.
4. Once set_kwarg_attrs became simpler, I decided to just put the code directly into the constructors rather than as a function. (this is perhaps dubious in our normal spirit of reuse...)
5. There are a few places that still need to pass in values extracted from dictionaries into function argument lists. Here it is more important to extract them more carefully, but this can be done inline. I changed the method name from construct_kwargs_from_dict to filter_kwargs as this is more what it does and makes the name shorter.

[connoramoreno]

These code changes are pretty slick; thanks for streamlining things @gonuke. I just have a question about exports in parastell.py and some requests regarding docstrings. I'll try to push a commit to this PR for the docstrings.

[connoramoreno]

In my commit, I changed export_dir in export_full_model to a keyword argument to match our export method convention, in case Python users would like to use build_full_model and/or export_full_model.

However, using these methods requires inputs to be defined in dictionary format, which is different from how we've defined inputs for the rest of the Python API. It might be best just to enforce one canonical input format for Python users, and maintain that build_full_model and export_full_model be intended for the command-line workflow.

[gonuke]

However, using these methods requires inputs to be defined in dictionary format, which is different from how we've defined inputs for the rest of the Python API. It might be best just to enforce one canonical input format for Python users, and maintain that build_full_model and export_full_model be intended for the command-line workflow.

Right. My first pass just made these global functions and took stellarator as an argument rather than member functions. We could revert to that - what do you think? Note: as I work towards the NWL mode, these two functions get moved to a method that is just for the main mode. (see #100 as a work in progress)

[connoramoreno]

I think making build_full_model and export_full_model global functions makes sense. We can then do something similar for the NWL PR. I can make that change to this PR quickly.

[gonuke]

@connoramoreno These all have positional arguments - shouldn't we call them with those arguments extracted from the dictionaries explicitly here?

[connoramoreno]

Defining the positional arguments and unpacking the respective dictionaries in the same function call defines those positional arguments twice, causing a TypeError

[connoramoreno]

Just unpacking the dictionaries will appropriately set the positional arguments.

[connoramoreno]

If we want to explicitly set positional arguments, we'll need to do something similar to what I had before where secondary dictionaries are created for the kwargs

[gonuke]

OK - Is there an error if one of the positional arguments is missing? (Maybe we should add a test for that at some point)

[connoramoreno]

Yes, if a positional argument is missing, a TypeError will be thrown.

[connoramoreno]

E.g., I removed poloidal_angles from config.yaml to test:

File "/home/camoreno/parastell/parastell/parastell.py", line 545, in build_full_model
    stellarator.construct_invessel_build(**invessel_build)
TypeError: Stellarator.construct_invessel_build() missing 1 required positional argument: 'poloidal_angles'

[gonuke]

I think this is ready to merge, then...

[connoramoreno]

Agreed!

[connoramoreno]

Looks good now. Thanks @gonuke