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 temporal attribute to RasterBlock + base Clip logic on it #117

Merged
merged 18 commits into from
Aug 19, 2024

Conversation

caspervdw
Copy link
Collaborator

@caspervdw caspervdw commented Aug 13, 2024

I added the RasterBlock.temporal attribute with a default implementation that guesses whether the raster is temporal based on timedelta and period.

We can gradually improve this by providing temporal from the various sources.

I went through the blocks and fixed some low-hanging fruit for propagating temporal through the operations.

Also: many deprecation warnings that I solved with pinning versions. See issues #118 , #119 , #120

@caspervdw caspervdw self-assigned this Aug 13, 2024
Copy link
Contributor

@benvanbasten-ns benvanbasten-ns left a comment

Choose a reason for hiding this comment

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

Seems ok, but is NONTEMPORAL_TIMEDELTA still required?

Copy link
Contributor

@arjanverkerk arjanverkerk left a comment

Choose a reason for hiding this comment

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

Nice to finally get the temporal attribute on the blocks. I had one comment about a docstring. Otherwise approved.

else:
return timedeltas[0]

@property
def temporal(self):
"""If any of the sources is temporal, the result is temporal."""
Copy link
Contributor

Choose a reason for hiding this comment

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

This is enforced by the init(), but maybe not the most adequate description for this property.

@caspervdw caspervdw merged commit f7c8f57 into master Aug 19, 2024
1 check 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.

3 participants