Closed willu47 closed 1 year ago
This looks so cool, @willu47!! My schedule is pretty tight for the rest of this week, but I should have time early next week to review this and give feedback!
Hi @willu47! I am so sorry this took me nearly 2 months to review.
This is a really excellent addition! I ran through the examples you posted in the Python API
documentation and was able to successfully complete them all! I have three notes on the PR:
The API functions have positional arguments ordered so the configuration file is passed first. The CLI functions have the configuration file as the last positional argument. Would it make sense to coordinate these, so the config file is always either the first or last positional argument?
The current API functions include convert(...)
, convert_results(...)
, read(...)
, and write(...)
. Would it make sense to also include a function to directly read in solver results (such as a read_results(...)
function that returns the dictionary of dataframes)?
The documentation page of Python API
is great! I was able to very easily follow your examples. I am just wondering about how to coordinate these examples with the CLI examples on the Examples
page. Do you think it would make sense to try and combine these into one page? I am thinking that may be a little overwhelming though. Or maybe we try to mirror the examples on the CLI page to the Python API page. Or maybe I am just overthinking this and doing as you have already done by linking the module reference is good!
Wherever we land on these points, I am happy to help address them if needed! :)
Hi @trevorb1 - see the latest commits for adding a read_results
function and refactoring the CLI to now use the new Python interface, resulting in a cleaner separation.
I have also "fixed" the awkward flags in the otoole results
CLI, replacing them with two positional arguments for the input data format and path. This means that input data is now required for result processing, but that all input formats will be read in through the otoole's own read
interface (and it will be easier to support new formats as they are added).
I haven't moved the config arguments around, as I feel that CLI and Python interfaces will be used separately, and the documentation is such that it is easy to follow e.g. docstrings for Python interface, and the integrated help dialog for the CLI.
In previous versions of otoole, the internal read and write strategies were exposed to the user. This was not ideal as it meant that the user had to know the internal structure of the model. This commit adds a public API to
convert
,read
,write
andconvert_results
methods. These methods are used to read, write and convert between different file formats for input data, and convert from solution files to a folder of csv files for results.Description
This PR also:
Issue Ticket Number
181
Documentation
Added a documentation page "Python API" with a short outline of the new functions, and links to the module documentation. Docstrings are filled out in detail.