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

Group by enhancements #1157

Merged
merged 3 commits into from Jul 19, 2021
Merged

Group by enhancements #1157

merged 3 commits into from Jul 19, 2021

Conversation

uchchwhash
Copy link
Member

@uchchwhash uchchwhash commented Jul 16, 2021

Reason for this pull request

Basically #1153. Please test to see if this is fit for purpose @robbibt.

Proposed changes

@uchchwhash uchchwhash requested review from omad and robbibt July 16, 2021 13:30
@codecov
Copy link

codecov bot commented Jul 16, 2021

Codecov Report

Merging #1157 (66bfaff) into develop (37b8498) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1157      +/-   ##
===========================================
+ Coverage    93.91%   93.93%   +0.01%     
===========================================
  Files          102      102              
  Lines        10293    10301       +8     
===========================================
+ Hits          9667     9676       +9     
+ Misses         626      625       -1     
Impacted Files Coverage Δ
datacube/virtual/impl.py 84.40% <ø> (ø)
datacube/api/core.py 97.11% <100.00%> (+0.24%) ⬆️
datacube/api/query.py 94.81% <100.00%> (+0.25%) ⬆️
datacube/utils/masking.py 100.00% <100.00%> (ø)
datacube/utils/math.py 100.00% <100.00%> (ø)
datacube/virtual/transformations.py 79.39% <100.00%> (ø)

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 37b8498...66bfaff. Read the comment docs.

@robbibt
Copy link
Contributor

robbibt commented Jul 19, 2021

Thanks so much @uchchwhash , will test this this week!

Copy link
Contributor

@robbibt robbibt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works nicely: I've tested it with the following custom grouper designed to prioritise Landsat 8 > Landsat 5 > Landsat 7 geomedian, and it works great:

def _extract_time_from_ds(ds: Dataset) -> datetime.datetime:
    return normalise_dt(ds.center_time)

def sort_by_platform(ds):
    order = {'LANDSAT_5': 2, 'LANDSAT_7': 3, 'LANDSAT_8': 1}
    return order[ds.metadata.platform]


platform_grouper = GroupBy(dimension='time',
                           group_by_func=_extract_time_from_ds,
                           units='seconds since 1970-01-01 00:00:00',
                           sort_key=sort_by_platform)

As expected, I now get correct dates returned in the output xarray:

image

And my output nicely prioritises the correct sensors (e.g. no Landsat 7 striping from 2013 onward where both Landsat 8 and Landsat 7 data available):

image

@uchchwhash uchchwhash merged commit 3765dfb into develop Jul 19, 2021
@uchchwhash uchchwhash deleted the group-by-enhancements branch July 19, 2021 07:11
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.

Unexpected use of GroupBy's sort_key to label output dimensions when applying custom grouping
2 participants