Improve cf conformance 5.6 grid mappings and projections #976

leilabbb · 2022-12-09T22:11:43Z

No description provided.

related to issue#950 [ioos#950] updated the function get_grid_mapping_variables to parse the attribute variables when it contains more than one string.

related to issue # 950 [ioos#950] Mocked a new .nc file to test the grid_mapping requirements and recommendations

fixed the for loop of the get_grid_mapping_variables function

in cf_1_7.py - added "geographic_crs_name" to the function def _check_gmattr_existence_condition_ell_pmerid_hdatum(self, var): - Modified function def check_grid_mapping(self, ds): in cfutil.py - modified the function def grid_mapping_variables(ds) - added the function def get_grid_mapping_variables_coordinates(ds): in test_cf.py - modified the function def test_check_grid_mapping(self):

codecov · 2022-12-09T22:39:30Z

Codecov Report

Attention: Patch coverage is 73.03371% with 24 lines in your changes are missing coverage. Please review.

Please upload report for BASE (main@640ecb2). Learn more about missing BASE report.

Files	Patch %	Lines
compliance_checker/cf/cf_1_7.py	68.65%	13 Missing and 8 partials ⚠️
compliance_checker/cfutil.py	86.36%	0 Missing and 3 partials ⚠️

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #976   +/-   ##
=======================================
  Coverage        ?   81.55%           
=======================================
  Files           ?       24           
  Lines           ?     5243           
  Branches        ?     1262           
=======================================
  Hits            ?     4276           
  Misses          ?      655           
  Partials        ?      312

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

benjwadams · 2023-01-05T00:02:37Z

@leilabbb, could you address the test failures?

benjwadams · 2023-01-05T00:04:42Z

compliance_checker/tests/test_cf.py

-            "§5.6 Horizontal Coordinate Reference Systems, Grid Mappings, Projections"
-        )
-        assert all(r.name == expected_name for r in results.values())
+    # def test_check_grid_mapping(self):


Please remove commented function

commented function deleted.

compliance_checker/cf/cf_1_7.py

benjwadams · 2023-01-05T00:06:45Z

compliance_checker/cf/cf_1_7.py

@@ -501,6 +504,7 @@ def _check_gmattr_existence_condition_ell_pmerid_hdatum(self, var):
                    "reference_ellipsoid_name",
                    "prime_meridian_name",
                    "horizontal_datum_name",
+                    "geographic_crs_name",


Add test coverage for added functionality.

in progress ...

do not understand " not covered by test" comment:

there is a test "_check_grid_mapping_attr_condition" that evaluates the 'geographic_crs_name' in the file

removed commented function

compliance_checker/cf/cf_1_7.py

removed "print( )" to suppress output, it is needed only for debugging. removed the IF conditional statement and used the tests' results from _check_gmattr_existence_condition_ell_pmerid_hdatum and _check_gmattr_existence_condition_geoid_name_geoptl_datum_name

benjwadams · 2023-03-28T17:41:33Z

Can you check the test failures on this PR?

using regex I have replaced an inner loop with: grid_mapping_variable_list.append(regex.findall(r"(\w+)",item.grid_mapping))

added the "else" clause to the "if not" statement on line 740

benjwadams · 2023-05-16T15:11:09Z

compliance_checker/cf/cf_1_7.py

+
+            # [5/9] The grid mapping variables must have the grid_mapping_name attribute. 
+            # The legal values for the grid_mapping_name attribute are contained in Appendix F.
+            if "grid_mapping_name" not in var.ncattrs():


if not hasattr(var, "grid_mapping_name") would be more Pythonic.

agreed... changed

benjwadams · 2023-05-16T15:23:26Z

compliance_checker/cf/cf_1_7.py

+
+            # [6/9] The data types of the attributes of the 
+            # grid mapping variable must be specified in Table 1 of Appendix F.
+            type_map = {"S": "str" , "N": "float64"}


The typecheck for N is likely too strong. N simply means that the value has a numeric type, not just floating point.

agreed.... changed to:
type_map = {"S": "str", "N": ["float64", "int"]}

benjwadams · 2023-05-16T15:24:36Z

compliance_checker/cf/cf_1_7.py

-                        test_ctx.score += 1
-                    test_ctx.out_of += 1
-
+                        test_ctx.out_of += 1                                   


Same comment about hasattr mentioned above.

agreed ... changed

benjwadams · 2023-05-16T15:26:11Z

compliance_checker/cf/cf_1_7.py

-            elif len_vdatum_name_attrs == 1:
+            vert_datum_attrs = possible_vert_datum_attrs.intersection(var.ncattrs())                             
+
+            if  exist_cond_1[0] == True and vert_datum_attrs:              


Just use if exist_cond_1[0] and vert_datum_attrs if exist_cond[0] is expected to be boolean.

benjwadams

Please also fix and/or change unit tests as necessary.

benjwadams

EDIT: already mentioned

…-Projections

leilabbb

all comments were resolved except one that i am not sure about "Add test coverage for added functionality."

leilabbb · 2023-02-27T15:23:59Z

compliance_checker/tests/test_cf.py

-            "§5.6 Horizontal Coordinate Reference Systems, Grid Mappings, Projections"
-        )
-        assert all(r.name == expected_name for r in results.values())
+    # def test_check_grid_mapping(self):


commented function deleted.

leilabbb · 2023-02-27T15:30:44Z

compliance_checker/cf/cf_1_7.py

@@ -501,6 +504,7 @@ def _check_gmattr_existence_condition_ell_pmerid_hdatum(self, var):
                    "reference_ellipsoid_name",
                    "prime_meridian_name",
                    "horizontal_datum_name",
+                    "geographic_crs_name",


do not understand " not covered by test" comment:

there is a test "_check_grid_mapping_attr_condition" that evaluates the 'geographic_crs_name' in the file

leilabbb · 2024-07-16T19:07:37Z

compliance_checker/cf/cf_1_7.py

+
+            # [5/9] The grid mapping variables must have the grid_mapping_name attribute. 
+            # The legal values for the grid_mapping_name attribute are contained in Appendix F.
+            if "grid_mapping_name" not in var.ncattrs():


agreed... changed

leilabbb · 2024-07-16T19:10:32Z

compliance_checker/cf/cf_1_7.py

+
+            # [6/9] The data types of the attributes of the 
+            # grid mapping variable must be specified in Table 1 of Appendix F.
+            type_map = {"S": "str" , "N": "float64"}


agreed.... changed to:
type_map = {"S": "str", "N": ["float64", "int"]}

leilabbb · 2024-07-16T19:11:16Z

compliance_checker/cf/cf_1_7.py

-                        test_ctx.score += 1
-                    test_ctx.out_of += 1
-
+                        test_ctx.out_of += 1                                   


agreed ... changed

leilabbb added 5 commits November 15, 2022 12:58

Update cfutil.py

bb491a5

related to issue#950 [ioos#950] updated the function get_grid_mapping_variables to parse the attribute variables when it contains more than one string.

Update test_cf.py

9c0b9ca

related to issue # 950 [ioos#950] Mocked a new .nc file to test the grid_mapping requirements and recommendations

Update cfutil.py

f1d05a4

fixed the for loop of the get_grid_mapping_variables function

Update test_cf.py

250c3d6

benjwadams requested changes Jan 5, 2023

View reviewed changes

Update test_cf.py

be02e31

removed commented function

benjwadams requested changes Jan 24, 2023

View reviewed changes

compliance_checker/cf/cf_1_7.py Outdated Show resolved Hide resolved

Update cf_1_7.py

9286cf5

removed "print( )" to suppress output, it is needed only for debugging. removed the IF conditional statement and used the tests' results from _check_gmattr_existence_condition_ell_pmerid_hdatum and _check_gmattr_existence_condition_geoid_name_geoptl_datum_name

leilabbb added 2 commits May 1, 2023 16:40

Update cf_1_7.py

0f0fa24

using regex I have replaced an inner loop with: grid_mapping_variable_list.append(regex.findall(r"(\w+)",item.grid_mapping))

Update cf_1_7.py

177513a

added the "else" clause to the "if not" statement on line 740

benjwadams requested changes May 16, 2023

View reviewed changes

benjwadams and others added 4 commits May 21, 2024 14:24

Merge branch 'main' into Improve-CF-Conformance-5.6-Grid-Mappings-and…

26e5287

…-Projections

Fixup unused parameter name in cfutil get_grid_mapping_variables

84a08ec

FIXUP: More undef fixes

e5ba8be

FIXUP: Add missing regex module

579a018

ocefpaf deleted the branch ioos:main July 16, 2024 17:39

ocefpaf closed this Jul 16, 2024

leilabbb commented Jul 16, 2024

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve cf conformance 5.6 grid mappings and projections #976

Improve cf conformance 5.6 grid mappings and projections #976

leilabbb commented Dec 9, 2022

codecov bot commented Dec 9, 2022 •

edited

Loading

benjwadams commented Jan 5, 2023

benjwadams Jan 5, 2023 •

edited

Loading

leilabbb Feb 27, 2023

benjwadams Jan 5, 2023

leilabbb Jan 24, 2023

leilabbb Feb 27, 2023

benjwadams commented Mar 28, 2023

benjwadams May 16, 2023

leilabbb Jul 16, 2024

benjwadams May 16, 2023

leilabbb Jul 16, 2024

benjwadams May 16, 2023

leilabbb Jul 16, 2024

benjwadams May 16, 2023

benjwadams left a comment

benjwadams left a comment •

edited

Loading

leilabbb left a comment

leilabbb Feb 27, 2023

leilabbb Feb 27, 2023

leilabbb Jul 16, 2024

leilabbb Jul 16, 2024

leilabbb Jul 16, 2024

Improve cf conformance 5.6 grid mappings and projections #976

Improve cf conformance 5.6 grid mappings and projections #976

Conversation

leilabbb commented Dec 9, 2022

codecov bot commented Dec 9, 2022 • edited Loading

Codecov Report

benjwadams commented Jan 5, 2023

benjwadams Jan 5, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjwadams commented Mar 28, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

benjwadams left a comment

Choose a reason for hiding this comment

benjwadams left a comment • edited Loading

Choose a reason for hiding this comment

leilabbb left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Dec 9, 2022 •

edited

Loading

benjwadams Jan 5, 2023 •

edited

Loading

benjwadams left a comment •

edited

Loading