Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Integrate time coordinate with mobility functions #92

Merged
merged 7 commits into from
Feb 1, 2022

Conversation

elbeejay
Copy link
Contributor

@elbeejay elbeejay commented Jan 15, 2022

This PR does the time integration for the mobility functions (relates to #75).

We adopt the same syntax as the section cuts, and simply offer both a basevalues and basevalues_idx input option to set the base maps by either their time, or time index. The same is offered for window and window_idx where the time lag over which mobility functions are computed can be set by either a dimensional value or value representing number of indices.

The dimension of the returned object from the mobility functions should be flexible, so it will correspond to the first dimension of the 3-D xarray (if provided). This would allow someone to apply these methods to any arbitrary model output or input data. Overall a lot of input type handing and conversion goes on to accept inputs that are lists of arrays, or 3-D arrays (and xarray counterparts).

The channel_presence function has also been updated to keep track of dimensions and accept a wider set of input types.

Tests and doc-strings have been updated to reflect changes, and an example has been added to the main page of the mobility API documentation as the computation required to generate a bunch of masks for each individual function docstring seemed excessive. For the singular example we compute land masks and channel masks for 10 of the golfcube time slices - I don't think this is too excessive for the docs build, but we can scale that back a bit if needed.

of note: the masking functionality preserves the time dimension when using __init__ pathways but not via the static methods, this has been noted in #75 and will be the subject of a future PR. Mobility calculation using the _idx inputs will still work if the time dimension is messed up.

edit: additional documentation addresses #29


old notes

This PR will eventually fully integrate the time coordinate with the mobility functions...

to do

  • switch mobility functions to initialize xarrays instead of ndarrays - inputs already sanitized to become dimensional xarrays if they are not already
  • update docstrings to support new set of inputs (basevalues, basevalues_idx, window, window_idx)
  • update and add more tests to cover the new cases when input values are not indices but actual time values
  • create some sort of example in documentation using the golf-example and the mobility functions (maybe in index page so the set of masks are only generated once)
  • flexible coordinate (aka dont assume it is 'time')

This commit revises the input checking for the mobility functions.
Now the input checker can digest time-coordinate inputs or
time-index values, and it spits out dimensioned xarray objects
as well as converted (if needed) indices for the time values.

Revises the testing to get things working. Still need to do
the doc-strings and change the actual mobility functions to
stop returning arrays and return dimensioned xarrays...
@codecov
Copy link

codecov bot commented Jan 16, 2022

Codecov Report

Merging #92 (34fb8d0) into develop (2e9c28a) will increase coverage by 0.02%.
The diff coverage is 92.37%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop      #92      +/-   ##
===========================================
+ Coverage    88.23%   88.25%   +0.02%     
===========================================
  Files            9        9              
  Lines         3060     3109      +49     
===========================================
+ Hits          2700     2744      +44     
- Misses         360      365       +5     
Impacted Files Coverage Δ
deltametrics/mobility.py 94.50% <92.37%> (-1.74%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2e9c28a...34fb8d0. Read the comment docs.

@elbeejay elbeejay marked this pull request as ready for review January 17, 2022 02:07
@elbeejay elbeejay requested a review from amoodie January 17, 2022 02:07
@elbeejay elbeejay merged commit 5b2d91a into sandpiper-toolchain:develop Feb 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant