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

34 morton sorting #56

Merged
merged 12 commits into from
Nov 21, 2023
Merged

34 morton sorting #56

merged 12 commits into from
Nov 21, 2023

Conversation

vanlankveldthijs
Copy link
Contributor

Added functions to STM for creating order attribute per point (implemented as Morton code).
Commented these functions.
Added several unit tests for these functions.

thijsvl and others added 12 commits August 15, 2023 11:03
        Note that for application to an stm, we should still check whether we can specify to mortonorder() the dimensions to sort on. Otherwise, we could temporarily rename lat-lon to Y-X before sorting.
… using pymorton based on pixel coordinates.

This should be made a lazily executing function in stm.py
Tests:
* existence of attribute before evaluation.
* correctness of order after sorting manually (integers).
* correctness of order after sorting using function (integers).
* correctness of order aftre sorting (float values with(out) scaling).
Copy link

sonarcloud bot commented Nov 15, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

100.0% 100.0% Coverage
0.0% 0.0% Duplication

@rogerkuou
Copy link
Member

Hi @vanlankveldthijs, thanks a lot for the implement!
After investigation, I think the list comprehention of the get order should be fine, since apply_gufunc apply chunk-wisely. Then the list conprehension should be good per chunk.

I did several change:

  1. added pymorton as a dependency
  2. moved the WIP example notebook to experiment
  3. when calling apply_gufunc, I only used .data for coordinate vectores. If not they come as xr.DataArray with redundant coords.
  4. Formatted with ruff

To me it's all good to merge. Would you like to give a final check?
Also created an issue #57 for documentation.

@rogerkuou
Copy link
Member

Hi @vanlankveldthijs , I will merge this maybe?

@rogerkuou rogerkuou merged commit bd0761c into main Nov 21, 2023
18 checks passed
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.

None yet

2 participants