DAS-2238 - updates to enable 3D variables without dimension scales to… #27
Conversation
sudha-murthy
requested review from
D-Auty,
flamingbear,
joeyschultz,
lyonthefrog,
owenlittlejohns and
vutrannasa
| ): | ||
| dimension_array_names = get_dimension_array_names(varinfo, variable_name) | ||
| dimension_array_names = get_dimension_array_names_from_coordinates( | ||
| varinfo, variable_name |
There was a problem hiding this comment.
The comment says if variable does not have coordinates (len = 0) but the code now will be get_dimension_arraay_names_coordinates ? I think a better name is get_default_dimension_names
There was a problem hiding this comment.
but the variable itself is a coordinate. So we are using the coordinate to name it. Will update the comment
There was a problem hiding this comment.
updated comment - b9d3fe6
There was a problem hiding this comment.
Sorry that the logic here still confuses me, and yes coordinates have coordinates - so that is a bit confusing. So...
# Given variable has coordinates: use latitude coordinate to define variable spatial dimensions.
# Given variable variable has no coordinate attribute itself, but is itself a coordinate (latitude or longitude): use as a coordinate
In both cases above - function should be: create_dimension_array_names_for_coorrdinate.
There was a problem hiding this comment.
updated - 7614f4c
|
Overall, the changes look good! All unit tests and the harmony-in-a-box test passed. Once my minor comments are resolved I should be able to approve this. |
hoss/exceptions.py
Outdated
| class InvalidDimensionNames(CustomError): | ||
| """This exception is raised when the granule file included in the input | ||
| request is not the nominal dimension order which is 'y,x'. | ||
There was a problem hiding this comment.
Looks like a cloned comment that I suspect is not accurate. It is the same description as the next exception UnsupportedDimensionOrder. I can't tell if this exception is referenced anywhere. One search shows it used in coordinate_utilities.py line 160, but that line, when I look in that file, shows a different exception raised???
There was a problem hiding this comment.
Sorry about that. I copied over the function and missed to update the comment.
updated - 7614f4c
| ): | ||
| dimension_array_names = get_dimension_array_names(varinfo, variable_name) | ||
| dimension_array_names = get_dimension_array_names_from_coordinates( | ||
| varinfo, variable_name |
There was a problem hiding this comment.
Sorry that the logic here still confuses me, and yes coordinates have coordinates - so that is a bit confusing. So...
# Given variable has coordinates: use latitude coordinate to define variable spatial dimensions.
# Given variable variable has no coordinate attribute itself, but is itself a coordinate (latitude or longitude): use as a coordinate
In both cases above - function should be: create_dimension_array_names_for_coorrdinate.
hoss/coordinate_utilities.py
Outdated
| if len(dimension_names) >= 2: | ||
| return dimension_names | ||
|
|
||
| # getting dimension names from coordinates |
There was a problem hiding this comment.
# creating dimension names from coordinates
There was a problem hiding this comment.
updated - 7614f4c
|
@D-Auty @joeyschultz - I think all the changes have been updated. Let me know if anything is still missing |
hoss/coordinate_utilities.py
Outdated
|
|
||
|
|
||
| def get_dimension_array_names(varinfo: VarInfoFromDmr, variable_name: str) -> str: | ||
| def get_dimension_array_names_from_coordinates( |
There was a problem hiding this comment.
`def create_dimension_array_names_for_coorrdinate.
There was a problem hiding this comment.
updated - 601344d
…to create_spatial_dimension_names_from_coordinates
3 participants
Description
-SMAP L3 collections do not have CF compliance. No dimension arrays or cos are present. This update enables spatial subsetting for 3D variables . The 3D dimensions are configured in hoss_config.json
Jira Issue ID
DAS-2238
Local Test Steps
cd harmony-opendap-subsetter
Run the following:
bin/build-image
bin/build-test
bin/run-test
Unit tests should pass
The following requests should pass
http://localhost:3000/C1268617120-EEDTEST/ogc-api-coverages/1.0.0/collections/parameter_vars/coverage/rangeset?forceAsync=true&granuleId=G1268617167-EEDTEST&subset=lat(60%3A85)&subset=lon(-70%3A-10)&variable=Freeze_Thaw_Retrieval_Data_Global%2Fsurface_flag,Freeze_Thaw_Retrieval_Data_Global%2Ftransition_direction&skipPreview=true
If you check the logs - you should have the following
variables_with_ranges: /Freeze_Thaw_Retrieval_Data_Global/latitude[][0:26][294:455], /Freeze_Thaw_Retrieval_Data_Global/longitude[][0:26][294:455], /Freeze_Thaw_Retrieval_Data_Global/transition_direction[0:26][294:455]
PR Acceptance Checklist
CHANGELOG.mdupdated to include high level summary of PR changes.docker/service_version.txtupdated if publishing a release.