murphyqm / pytesimal

Model the conductive cooling of small planetary bodies with temperature-dependent material properties
MIT License
1 stars 0 forks source link

Documentation #33

Open murphyqm opened 3 years ago

murphyqm commented 3 years ago

Add full documentation including community guidelines, list here.

andreww commented 3 years ago

The big bit of work there is the "Functionality documentation", but from what I can see you have most of this in the doc strings (which look good). One way to use these is with pydoc (scripting something to run "pydoc -w $file" on each python file will generate a load of HTML documentation). However, I suspect there are much better ways to do this via http://docs.readthedocs.io

andreww commented 3 years ago

Also, a very useful form of documentation is an executable notebook. Once the packaging is done this is easy to set up via binder. Basically you can build a URL that embeds the notebook location on github, following that link gives the reader a running notebook to play with.

murphyqm commented 3 years ago

Have Sphinx up and running, auto-building docs from docstrings. Issues with the mantle_timestepping module as this contains relative imports (it imports core module and mantle properties module); however, on the to-do list is changing the way this function uses the core object (moving instantiation out of the timestepping function and instead feeding in a calleable argument), and will do similar with the mantle_properties object (improves modularity) too, which results in not requiring relative imports. Can look into fixing the relative imports issue more if when all the refactoring and tidying up is finished and it ends up still being a problem.

Have not yet set up a github workflow to rebuild docs when changes are pushed to the repo, and haven't yet linked it to read the docs. Need to rewrite/add detail to some doc strings, and remove comments that are accidentally being pulled into the documentation.

murphyqm commented 3 years ago

Also sphinx has an extension that builds a html gallery of examples from a defined directory of python scripts; can point this to jupyter nb files so they are included tidily in the documentation (as well as having a live version on binder) - https://sphinx-gallery.github.io/stable/index.html

andreww commented 3 years ago

Nice! It looks like that's how Matplotlib makes their examples.

murphyqm commented 3 years ago

Doc issues for pytesimal package imports

When certain packages (including pytesimal) are imported, sphinx autodoc fails, because these packages haven't been added to it's requirements.txt.

Commenting out from . import pytesimal.blah from the and then building html docs works to create a correct local html version of the docs, however read the docs builds from the source script in the repository, so this fix doesn't work for the hosted version of the docs.

Going to use their mock functionality to fake imports of certain dependencies (more info on their FAQ page). Not sure if this will work with the pytesimal package... but hopefully. There's probably a way to define the package name somewhere in the sphinx config files, so it just accepts a from . import statement.

andreww commented 3 years ago

I'm putting any minor documentation issues on this branch: https://github.com/murphyqm/pytesimal/tree/amw-docs - I'll generate a pull request once I think I've found them all.

andreww commented 3 years ago

Probably need to go through all the docstrings to check:

We probably also need to think about how to explain the coupling in a useful way ... this will be needed in the paper and (possibly) in an example.

murphyqm commented 3 years ago

Just combing through docstrings now, adding units. Does the below sound sensible for specifying call signatures for the material properties in the discretisation function, in the numerical_methods module? I essentially modeled it off the descriptions you wrote for top_mantle_bc and bottom_mantle_bc in the same function, but wasn't sure if I've formatted the call sigs correctly (since it's referencing a method of an object as opposed to a standalone function).

cond : callable
        Callable function or method that defines the mantle conductivity. The
        calling signature is cond.getk(temperatures[radial_index,
        timestep_index]), where temperatures is the temperatures array,
        radial_index is the row index of the radius, and timestep_index is the
        column index of the timestep, that define the value in temperatures at
        which conductivity should be evaluated. The function must return a
        value for conductivity in in W m^-1 K^-1.
heatcap : callable
        Callable function or method that defines the mantle heat capacity. The
        calling signature is heatcap.getcp(temperatures[radial_index,
        timestep_index]), where temperatures is the temperatures array,
        radial_index is the row index of the radius, and timestep_index is the
        column index of the timestep, that define the value in temperatures at
        which heat capacity should be evaluated. The function must return a
        value for heat capacity in J kg^-1 K^-1.
dens : callable
        Callable function or method that defines the mantle density. The
        calling signature is dens.getrho(temperatures[radial_index,
        timestep_index]), where temperatures is the temperatures array,
        radial_index is the row index of the radius, and timestep_index is the
        column index of the timestep, that define the value in temperatures at
        which heat capacity should be evaluated. The function must return a
        value for density in kg m^-3.
andreww commented 3 years ago

Do you need to pass in objects here, or could you pass in the method itself as a callable? Looking at cond for example, do you ever use anything else from the object or do you only use the getk method?

If you need the whole object inside discretisation I would change cond : callable to cond : conductivity object and say that the argument has to be an instantiated object from the class (linking to the class documentation if possible). However, I think you give users more freedom if you only need a callable. In which case I would change the code to accept a callable (so you put cond.getk in the real argument list, without the brackets) and documentation just specifies that the callable must return the conductivity given a scalar temperature. You would then probably want to add a comment to the effect that it is convenient for this passed function to be the getk method from a conductivity object so that the user can easily set things up etc. I guess as you have cond.getkdT you'll either need the whole object, or will need to pass in two functions. I would probably follow the various numpy optimiser API and pass in two functions (you could always make the derivative optional and if it's not provided find the derivative numerically).