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

Add axis to apply_ragged and unpack_ragged #281

Merged
merged 7 commits into from
Sep 26, 2023

Conversation

milancurcic
Copy link
Member

This PR:

  • Adds axis to apply_ragged and unpack_ragged to specify unpacking/concatenation axis; default is 0, which is consistent with np.concatenate and np.split.
  • Fixes a few edge cases that didn't work before:
    • If you use a reduction function over a non-ragged axis, passing xr.DataArrays now works.
    • If you use a reduction function over a non-ragged axis and return more than one result (tuple), concatenation now works.
  • Unrelated to this PR, there was an outstanding issue with a test for coriolis_frequency that tested for a Warning that shouldn't be there. I don't know why tests were passing on main but this was failing for me. It's fixed.
  • Minor version bump to 0.23.0

Closes #274.

@milancurcic milancurcic added the enhancement New feature or request label Sep 22, 2023
clouddrift/analysis.py Outdated Show resolved Hide resolved
clouddrift/analysis.py Show resolved Hide resolved
clouddrift/analysis.py Show resolved Hide resolved
@milancurcic
Copy link
Member Author

@selipot Please check the docstring and merge when ready.

@selipot selipot merged commit 17c3b43 into Cloud-Drift:main Sep 26, 2023
10 checks passed
philippemiron pushed a commit to philippemiron/clouddrift that referenced this pull request Nov 16, 2023
* coriolis_frequency is not expected to warn

* Use np.split in unpack_ragged

* Add axis to apply_ragged and unpack_ragged

* Reorder tests

* Bump minor version

* Make apply_ragged work for various edge cases with reduction functions, reduced xr.DataArrays, and multi-variable results

* Expand and update the docstring
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Specify concatenation axis to apply_ragged
3 participants