MetPy icon indicating copy to clipboard operation
MetPy copied to clipboard

Add module to read/interpret NCEI storm database

Open gerritholl opened this issue 2 years ago • 7 comments

Description Of Changes

Add a module with functionality to read storms from the NCEI storm database for a requested period in time. The functions rely on pint-pandas to add unit information.

Checklist

  • [ ] Closes #xxxx
  • [x] Tests added
  • [x] Fully documented

gerritholl avatar Jul 09 '21 11:07 gerritholl

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jul 09 '21 11:07 CLAassistant

I've added API documentation, but when I make html it's not showing up in html/api/generated/metpy.io.html. What do I need to do to make the new module appear there? Should I import it from metpy.io.__init__.py? I'm not used to needing to do that in other packages, so maybe there's something particular in MetPy documentation building that I don't understand yet.

gerritholl avatar Jul 09 '21 16:07 gerritholl

Thanks for this contribution! I've yet to do a review, but am glad to see this coming in.

To see your new docstrings added to the generated docs, you can import and use our Exporter class and its @Exporter.export decorator from package_tools.py; you can see usage of this in the other modules. This will add any functions you decorate to storms.__all__ and make them "public" as far as MetPy's API and the docs are concerned. Then, yes, you will need to extend the top-level io(the name we consider public) by importing your module within io.__init__ and extending io.__all__ as you see done for the other modules there. Then you should be able to import your code directly from the public io space, e.g. from metpy.io import get_noaa_storms_for_period and your docs should appear!

dcamron avatar Jul 09 '21 17:07 dcamron

I don't know how to resolve the issue raised by the Codacy Static Code Analysis — it seems to arise from the magic concerning __all__ in src/metpy/io/__init__.py.

gerritholl avatar Jul 12 '21 12:07 gerritholl

@dcamron @dopplershift My PR has a dependency on pint-pandas, a package which I think should fit well with the MetPy philosophy of associating units with numbers. However, there appears to be no package in conda-forge, leading to failing tests. Is pint-pandas an acceptable dependency or should I rewrite my PR to not use pint-pandas (and therefore not associate units with numbers)?

gerritholl avatar Oct 18 '21 08:10 gerritholl

I'm not sure when the tests were run, but looking at them to see the error I get under "install dependencies":

Error: The log was not found. It may have been deleted based on retention settings.

Are the tests only Windows and MacOs? No Linux?

gerritholl avatar May 23 '22 14:05 gerritholl

@gerritholl My impression of pint-pandas it that it's not particularly mature (@jthielen may have a more updated opinion), so I'm very reticent to depend on it at this time. Our (admittedly sub-optimal) solution to this point has been to add a .units attribute that contains a dictionary mapping column names to a unit string so that they're available for use later (by e.g. pandas_dataframe_to_unit_array).

If you can resolve the conflict in ci/requirements.txt (also please add a version to fsspec for whatever is current) and add fsspec to setup.cfg, that will trigger tests to run again.

dopplershift avatar May 24 '22 17:05 dopplershift