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

merge_cubes description #379

Closed
soxofaan opened this issue Aug 2, 2022 · 2 comments · Fixed by #405
Closed

merge_cubes description #379

soxofaan opened this issue Aug 2, 2022 · 2 comments · Fixed by #405
Milestone

Comments

@soxofaan
Copy link
Member

soxofaan commented Aug 2, 2022

(here we are again, discussing merge_cubes' description 😄 )

a couple of notes on the current description of merge_cubes

The data cubes have to be compatible

I don't think we properly defined what "compatible" means. I also think the first sentence of the description here should be a more verbose version of the summary and at least mention the verb "merge".

A merge operation without overlap should be reversible with (a set of) filter operations for each of the two cubes.

I think it's a bit strange to have this as second sentence of the description. We haven't properly defined "overlap" yet. I guess the sentence tries to define "merge_cubes" as the inverse of "filter_*", but don't think that is very clear at the moment.

The process performs the join on overlapping dimensions, with the same name and type.

I think using the wording "join on a dimension" is not ideal because "join on" is classic relational database terminology and is closer to "concatenate" than "merge" (which is actually meant here)

An overlapping dimension has the same name, type, reference system and resolution in both dimensions, but can have different labels.

There is something wrong here: "A ... dimension has the same name ... in both dimensions". I guess what is meant here:

Overlapping dimensions have the same name, type, reference system and resolution, but can have different labels

But then: isn't overlap about having the same labels, so why does this talk about different labels?

One of the dimensions can have different labels, for all other dimensions the labels must be equal. If data overlaps, the parameter overlap_resolver must be specified to resolve the overlap.

  • It seems a bit arbitrary that only one dimension can have different labels, and all the others must be equal. E.g. as spatial dimensions usually come as a pair (x and y): isn't it allowed to merge to cubes from different bboxes?
  • "all other dimensions the labels must be equal" seems to imply that there is always overlap, while the next sentence contradicts this.
@soxofaan
Copy link
Member Author

soxofaan commented Aug 4, 2022

I think part of the confusion is because the current description makes it easy to mix up "dimension overlap" and "cube overlap". It should be explained how they relate to each other. And maybe it is easier to talk in terms of disjoint dimensions (a pair of corresponding dimensions with no common labels), instead of trying to explain everything in terms of overlap. Roughly:

  • if there is at least one pair of corresponding dimensions that are disjoint: the cubes are not overlapping -> overlap resolver is not necessary
  • otherwise the cubes are overlapping -> overlap resolver is necessary

@m-mohr
Copy link
Member

m-mohr commented Feb 3, 2023

I'm trying to capture all(?) points in #405.

m-mohr added a commit that referenced this issue Feb 3, 2023
m-mohr added a commit that referenced this issue Feb 3, 2023
m-mohr added a commit that referenced this issue Feb 3, 2023
@m-mohr m-mohr added this to the 2.0.0 milestone Mar 8, 2023
m-mohr added a commit that referenced this issue Mar 14, 2023
…clarifications (#405)

* `mask` and `merge_cubes`: The spatial dimensions `x` and `y` can now be resampled implicitly instead of throwing an error. #402

* Clarify descriptions #379

* Improve wording as suggested by @soxofaan

* Update merge_cubes.json

* Slim down description

* Default parameters of resample_cube_spatial apply
@m-mohr m-mohr closed this as completed Mar 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants