Update MinimalLib for Function Exposure: runReactants

[syedzayyan]

This is an exposure of runReactants features to minimal lib:

• a single function is exposed
• results are an array of JSMolLists

It passed the simple test I coded. First-time contributor, please go easy :).

[syedzayyan]

Hello! @greglandrum @ptosco

A silly question. I plan to port ScaffoldNetwork to MinimalLib as well. Would I need to open a new PR for that or shall I merge that into this PR?

[ptosco]

@syedzayyan Please submit a separate PR.
Also, I'd recommend to make building ScaffoldNetwork support optional through a CMake switch (mapping to a preprocessor macro, such as RDK_BUILD_MINIMAL_LIB_SCAFFOLDNETWORK) in order not to keep MnimalLib as minimal as it can be for non-interested users.

[syedzayyan]

Thank you for your prompt response! @ptosco

I'll go ahead and submit a separate PR for the ScaffoldNetwork feature, as suggested with the CMake switch for greater flexibility.

On another note, if you find some time, I'd love to get your input on this PR :).

[greglandrum]

We don't do error reporting straight to std::cerr.
I think you need to thrown an exception here, but @ptosco should suggest what he thinks is the best approach for error handling in minilib

[ptosco]

@syedzayyan I have submitted a PR against your branch with some suggested changes.

1. You do not need to check d_rxn against non-nullness as JSReaction cannot be constructed with a null ChemicalReaction.
2. @greglandrum is right about the fact that it is better to throw an exception rather than printing anything to std::cerr
3. I would not catch upstream exceptions
4. I would return a vector of JSMolList pointers and call it JSMolListList for consistency with StringList
5. I would delete all C++ objects in the test; while it is not terrilbly important to leak some memory in the test, I think it is good practice to write tests which cleanly delete objects on exit as they may serve as examples to other developers.

[syedzayyan]

Hello @ptosco ! Thanks very much for the suggestions. I was wondering why return a vector of JSMolList pointers instead, on point 4. The build fails when a pointer is returned and allow_raw_pointers() is not helping with the Implicitly binding raw pointers is illegal error.

[ptosco]

I tested this locally and it looks good to me.

[greglandrum]

I just (hopefully) fixed the merge conflicts. Will merge later this afternoon after the CI builds go through