Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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 GeometryIndex #7
Add GeometryIndex #7
Changes from 23 commits
74ec7c9
5acb0b2
61539a5
3b7e020
2f21e56
604f5fd
5be917a
3256992
ef16849
97c90f5
7b35e6c
be351e1
6cc503d
90aaf1a
8c0f83d
dbf3c7d
531e4cf
cbcaa51
fe1dada
c539562
1a84c23
1e945b7
cd1cb4e
dcc4a32
b8fbbd0
4da5b85
f01e33a
6d770d7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
Large diffs are not rendered by default.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can this situation when
label_array
hascrs
attribute actually happen? Since we cast all inputs to thenumpy.array
above, even if we have something like aGeometryArray
, we would strip it of CRS. Codecov marking l. 193 as untested seems to prove that.I think we need to check for
crs
oflabels
notlabel_array
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually it depends (note:
labels
is a dict andlabel
is the extracted value).label
is aGeometryArray
then yes we need to checklabel.crs
label
is axarray.Variable
orxarray.DataArray
wrapping aGeometryArray
(i.e., the._data
attribute), we need to checklabel_array.crs
The latter is not supported yet in Xarray, though, and we still need to figure out a more general way to extract CRS from various input types. So I suggest to remove the CRS check here and address that in a follow-up PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine with me. Let's move this to an issue to keep track.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow this is raising a TypeError. Cell 6 in the Quickstart notebook:
This bit seems to override our default with
None
We need to catch that and override in that case. Either here like in the code suggestion above or in
_format_crs
.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good catch, we can fix it here but I think this should rather be fixed in
xarray.core.formatting.inline_index_repr
? cc @keewis.This file was deleted.