search

columncolab / EMC2

Earth Model Column Collabratory
BSD 3-Clause "New" or "Revised" License
10 stars 7 forks source link

Statistics llnl #105

Closed JingjingTina closed 1 year ago

rcjackson commented 1 year ago

Could you please add, for each procedure you submitted, documentation as a docstring in the beginning of each procedure that shows what the code does and what the inputs and outputs are? For example:

`def calc_total_reflectivity(model, detect_mask=False): """ This method calculates the total (convective + stratiform) reflectivity (Ze).

Parameters
----------
model: :func:`emc2.core.Model` class
    The model to calculate the parameters for.
detect_mask: bool
    True - generating a mask determining signal below noise floor.

Returns
-------
model: :func:`emc2.core.Model`
    The xarray Dataset containing the calculated radar moments.
"""`
JingjingTina commented 1 year ago

Docstrings are added. Feel free to reach out if suggestions. Thank you!

rcjackson commented 1 year ago

Great job on the docs! We will want to have unit tests for your modules, but we will move that to an issue that we can both work on.

Linting caught a potential issue with your code. I've highlighted the problem that could cause it to throw an error.

rcjackson commented 1 year ago

I've moved a couple of the plotting routines to SubcolumnDisplay since they fit better there. I've also added tests and some more information to the notebook markdown comments.

The remaining work to do is to have the statistics modules return xarray Datasets instead of numpy arrays for easier plotting and greater consistency with the other EMC^2 modules. This I will raise as a separate issue to work on after the ARM Developer's Meeting.