Recursive as dictionary

[amarkpayne]

Motivation or Problem

The methods as_dict and make_object allow for any object that inherits from RMGObject to be easily saved to a yaml file and reconstructed from a yaml file, respectively. However, as currently written is RMG there is a limited recursion depth to how many objects, dictionaries, and lists can be nested inside of each other.

For example, consider an object that has an attribute that is a dictionary which itself contains another object. The desired affect of as_dict would be for the object nested inside of this dictionary to be expanded to a dictionary (i.e. for as_dict to be called on this object as well). However, the current implementation will return for this attribute a dictionary that contains the object itself rather than the expanded object.

Furthermore, many objects have to rewrite their as_dict and make_objects to function properly, even if they inherit from RMGObject. Ideally, newly created objects that inherit from RMGObject should have working as_dict and make_object methods without needing to overwrite these methods (at least more times than not).

Description of Changes

Inside the file where we define the RMGObject I have created method for recursively calling as_dict for all objects inside of a dictionary or list. I have also created a method for recursively calling make_object for all objects inside of a dictionary. Becasue these functions are recursive by calling themselves, the recursion limit for nested objects and iterables should be whatever the recursion stack limit of python is (very large).

While these functions can be called as standalone functions (potentially useful for scripting), I have used these functions to help re-write the as_dict and make_object methods of the RMGObject class.

With this, I have also cleaned-up the affected code, and made it so that the as_dict and make_object methods are only re-written when absolutely necessary (for example, see statmech rotations and torsions), which thanks to the new functions require very little code even when necessary. Note that this is usually necessary whenever passing all attributes to the init method causes issues.

Testing

I have ran the unit tests, and even successfully recreated an ArkaneSpecies object from a YAML file, but it is likely that there are affected classes that weren't properly unit tested. I am specifically concerned about the Wilhoit class, and the NASA class can probably be cleaned up further.

Reviewer Note

Because we have a few pull requests open or about to be opened that define new objects that will be saved to a YAML file, these PRs will require these changes, and thus this PR needs to be merged in first. For example, this affects #1577, #1641, and #1561

[alongd]

Thanks for this additions! Below are some comments and questions

[rwest]

Not had chance to review the code, but from the pull request description this sounds like a great improvement. 👏🏻

[amarkpayne]

Thanks @alongd for the timely review and great comments! I have addressed your comments and added fixup commits to add changes where I could. Let me know what you think. In the meantime I think it might be worthwhile if I add some additional unit testing to catch some of the points that you addressed.

[codecov]

Codecov Report

Merging #1643 into master will increase coverage by 0.02%.
The diff coverage is 0%.

@@            Coverage Diff             @@
##           master    #1643      +/-   ##
==========================================
+ Coverage   41.68%   41.71%   +0.02%     
==========================================
  Files         176      176              
  Lines       29374    29359      -15     
  Branches     6059     6053       -6     
==========================================
  Hits        12246    12246              
+ Misses      16259    16244      -15     
  Partials      869      869

Impacted Files	Coverage Δ
rmgpy/quantity.py	0% <0%> (ø)	⬆️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7c1f46c...13b1cb6. Read the comment docs.

[amarkpayne]

@alongd I have applied a few of the changes that we discussed offline and also added some unit tests. I also further refactored quantity, NASA/NASAPolynomial, and Wilhoit. With this I have added everything that I wanted to add for this PR, so this should be ready for one last round of review (and final changes if necessary).

[amarkpayne]

As we discussed offline, the fixup commits that I squashed are on the branch rad_w_fixup on official

[alongd]

Thanks for these recent additions!
Please see some comments below

[amarkpayne]

Thanks again @alongd and @mliu49 for the comments, I'll push the changes soon.

[amarkpayne]

I have added the last of the changes that @mliu49 and I discussed here and offline. @alongd let me know if there are any other changes. Otherwise if you approve I will update the branch and squash the fixup commits in preparation for merging.

[mliu49]

I think the fixups look good and can be squashed. I made a couple minor formatting comments, which you can go ahead and squash as well if you choose to fix them.

[amarkpayne]

I added your formatting changes and squashed all of the fixup commits. Thanks again @mliu49 !

[amarkpayne]

The builds are getting errors related to cairo. It looks like this package was updated on the RMG channel today. @mliu49 any idea if something went wrong with the new build?

[mliu49]

Sorry, I was playing around with updating a couple dependencies yesterday. The newest noarch cairocffi build by conda-forge apparently works on mac but not linux. The linux-64 build works though, so I just removed the noarch build.

[alongd]

Thanks for adding the tests!!

[amarkpayne]

The last round of fixes/additions has been rebased/autosquashed. Let me know if this is ready to merge or if there are any other changes we missed.

[alongd]

Thanks for this awesome contribution!

[amarkpayne]

Thanks @alongd and @mliu49 for all of the hard work you put into this PR!