search

ioos / ioos_metrics

Working on creating metrics for the IOOS by the numbers
https://ioos.github.io/ioos_metrics/
MIT License
2 stars 4 forks source link

Refactor full glider metrics #80

Closed ocefpaf closed 1 month ago

ocefpaf commented 1 month ago

This is a bit more efficient and we also fix a corner case for when the glider has a single data point in space and we cannot compute a track.

ocefpaf commented 1 month ago

@MathewBiddle Mary Solokas require this change to run the glider metrics script I sent her. Do you mind reviewing this one and, after the merge, tag a new release?

MathewBiddle commented 1 month ago

I'm confused on what function is for what. My expectation was ngdac_gliders() is the full search, including providing time and bounding box (see below). However, it seems that _ngdac_gliders() is that function. 

ioos_metrics.ioos_metrics.ngdac_gliders(min_time="2023-06-01T00:00:00", max_time="2023-11-30T23:59:59", min_lat=10, max_lat=42, min_lon=-99, max_lon=-50)
Traceback (most recent call last):
  File "C:\Users\Mathew.Biddle\programs\Miniforge\envs\ioos-metrics\Lib\site-packages\IPython\core\interactiveshell.py", line 3577, in run_code
    exec(code_obj, self.user_global_ns, self.user_ns)
  File "<ipython-input-7-f4b919fdf3b1>", line 1, in <module>
    ioos_metrics.ioos_metrics.ngdac_gliders(min_time="2023-06-01T00:00:00", max_time="2023-11-30T23:59:59", min_lat=10, max_lat=42, min_lon=-99, max_lon=-50)
TypeError: ngdac_gliders() got an unexpected keyword argument 'min_time'

I think the second cell in the glider notebook is what's confusing me. https://github.com/ioos/ioos_metrics/blob/main/notebooks/glider_metrics.ipynb Too many things with similar names.

Outside of that comment, the rest of these changes look good.

MathewBiddle commented 1 month ago

new release: https://github.com/ioos/ioos_metrics/releases/tag/v0.2.0a0

ocefpaf commented 1 month ago

I'm confused on what function is for what. My expectation was ngdac_gliders() is the full search, including providing time and bounding box (see below). However, it seems that _ngdac_gliders() is that function.

Some time ago we discussed that the default should be the fast one, that is why the full search is the hidden method. We can change that if you want.

I think the second cell in the glider notebook is what's confusing me. https://github.com/ioos/ioos_metrics/blob/main/notebooks/glider_metrics.ipynb Too many things with similar names.

We can make that one a first class citizen and name it ngdac_gliders_full, and rename ngdac_gliders_fast to just ngdac_gliders. The goal was to keep it as a private method until the glider group decided which one they wanted to use: gdutils, the new one that was brewing, or this implementation. I guess that it is decided now that we are using this version form now on.