Repository: ioos/compliance-checker
Pull request #1039 (2023-08-01 17:38:29)

Remove pkg resources
[pkg_resources is notoriously slow and problematic](https://github.com/pypa/setuptools/issues/926). This PR tries to remove it from compliance-checker.

codecov[bot]: ## [Codecov](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos) Report
> Merging [#1039](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos) (13875df) into [develop](https://app.codecov.io/gh/ioos/compliance-checker/commit/e456821eb59f9118e2ed688aac9fd2896b248478?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos) (e456821) will **increase** coverage by `0.04%`.
> The diff coverage is `72.22%`.

```diff
@@             Coverage Diff             @@
##           develop    #1039      +/-   ##
===========================================
+ Coverage    82.14%   82.18%   +0.04%     
===========================================
  Files           24       24              
  Lines         5170     5171       +1     
  Branches      1292     1292              
===========================================
+ Hits          4247     4250       +3     
+ Misses         623      622       -1     
+ Partials       300      299       -1     
```


| [Files Changed](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos) | Coverage Δ | |
|---|---|---|
| [compliance\_checker/acdd.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2FjZGQucHk=) | `80.71% <ø> (ø)` | |
| [compliance\_checker/base.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2Jhc2UucHk=) | `71.83% <ø> (ø)` | |
| [compliance\_checker/cf/cf\_1\_6.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2NmL2NmXzFfNi5weQ==) | `89.85% <ø> (ø)` | |
| [compliance\_checker/cf/cf\_1\_7.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2NmL2NmXzFfNy5weQ==) | `93.72% <ø> (ø)` | |
| [compliance\_checker/runner.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL3J1bm5lci5weQ==) | `74.35% <0.00%> (ø)` | |
| [compliance\_checker/cf/cf\_base.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2NmL2NmX2Jhc2UucHk=) | `82.77% <66.66%> (ø)` | |
| [compliance\_checker/cf/util.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2NmL3V0aWwucHk=) | `71.26% <66.66%> (ø)` | |
| [compliance\_checker/cfutil.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2NmdXRpbC5weQ==) | `80.85% <100.00%> (ø)` | |
| [compliance\_checker/ioos.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL2lvb3MucHk=) | `82.74% <100.00%> (ø)` | |
| [compliance\_checker/protocols/netcdf.py](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos#diff-Y29tcGxpYW5jZV9jaGVja2VyL3Byb3RvY29scy9uZXRjZGYucHk=) | `65.71% <100.00%> (+1.00%)` | :arrow_up: |
| ... and [1 more](https://app.codecov.io/gh/ioos/compliance-checker/pull/1039?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos) | |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=ioos)

ocefpaf: @benjwadams there is still one part of `pkg_resources`  that we in the plugins stuff, the `pkg_resources.working_set`, but that one is a bit tricker and I'll leave it to another PR. This one is ready to review.
ocefpaf: @benjwadams this is hurting me in a setup where I have tons of packages and pkg_resources really drags everything down. While we still have the plugins to deal with, for now, this already helps speed things up. Do you mind taking a look?
benjwadams: Hi, sorry for the delay.  Will have a look.
benjwadams: Hi, this should still work with Compliance Checker plugins, correct?
benjwadams: Just did a test in a pristine environment with `cc-plugin-glider` and it worked.   However, when installing via `pip`, it looks like `packaging` package is missing, so should probably be added to `requirements.txt`.
ocefpaf: > Just did a test in a pristine environment with `cc-plugin-glider` and it worked.

Cool. The plugin tests are also passing in the CIs. We do need to review the whole plugin system at some point though. But not now 😬 

> However, when installing via `pip`, it looks like `packaging` package is missing, so should probably be added to `requirements.txt`.

Done! I guess some other dependency is pulling it in the CIs b/c it did not fail there.
ocefpaf: The glider and ugrid plugin failures are an issue upstream I'll try to tackle them this week.
ocefpaf: With https://github.com/ioos/cc-plugin-glider/pull/42 and https://github.com/ioos/cc-plugin-ugrid/pull/19 this one is green again!
jcermauwedu: What is the status of trying to include this PR?  I look forward to the reduced warnings generated by the test suite.
ocefpaf: > What is the status of trying to include this PR? I look forward to the reduced warnings generated by the test suite.

I just rebased it to make it mergeable again.

@benjwadams do you want us to do more testing or are you good with this PR?
ChrisBarker-NOAA: I just duplicated this work in: #1059 -- not sure if I did a better or worse job. But it would be great if one or the other were merged. 
jcermauwedu: @ChrisBarker-NOAA @ocefpaf @benjwadams I know, I almost did it a 3rd time with this PR(#1056) to fix some of the python tests.  I had to undo those fixes when I remembered they were in this PR.

What can we do to help?
ChrisBarker-NOAA: What's the blocker to merging this now? Probably better than mine, as mine has a few checks failing -- might as well do this first, and then rebase mine and figure out what's up.


ocefpaf: > What's the blocker to merging this now? Probably better than mine, as mine has a few checks failing -- might as well do this first, and then rebase mine and figure out what's up.

I'm waiting for Ben's response today and, if he is to busy, I'll self-merge this one. Stay tuned...
ChrisBarker-NOAA: great, thanks!

-CHB


Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 9e13b76..f0f1a08 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -1,6 +1,6 @@
 repos:
 - repo: https://github.com/pre-commit/pre-commit-hooks
-  rev: v4.5.0
+  rev: v4.6.0
   hooks:
     - id: trailing-whitespace
       exclude: compliance_checker/tests/data
@@ -19,7 +19,7 @@ repos:
         - test_requirements.txt
 
 - repo: https://github.com/psf/black
-  rev: 24.1.1
+  rev: 24.3.0
   hooks:
   - id: black
     language_version: python3
@@ -31,7 +31,7 @@ repos:
 
 
 - repo: https://github.com/astral-sh/ruff-pre-commit
-  rev: v0.2.0
+  rev: v0.3.5
   hooks:
     - id: ruff
 
diff --git a/compliance_checker/cf/cf_1_6.py b/compliance_checker/cf/cf_1_6.py
index dd571c0..2e31f02 100644
--- a/compliance_checker/cf/cf_1_6.py
+++ b/compliance_checker/cf/cf_1_6.py
@@ -2764,12 +2764,7 @@ class CF1_6Check(CFNCCheck):
             ):
                 valid = False
                 reasoning.append(
-                    "Dimension {} of boundary variable (for {}) must have at least {} elements to form a simplex/closed cell with previous dimensions {}.".format(
-                        boundary_variable.name,
-                        variable.name,
-                        len(variable.dimensions) + 1,
-                        boundary_variable.dimensions[:-1],
-                    ),
+                    f"Dimension {boundary_variable.name} of boundary variable (for {variable.name}) must have at least {len(variable.dimensions) + 1} elements to form a simplex/closed cell with previous dimensions {boundary_variable.dimensions[:-1]}.",
                 )
             result = Result(
                 BaseCheck.MEDIUM,
diff --git a/compliance_checker/cf/cf_1_7.py b/compliance_checker/cf/cf_1_7.py
index e47ca24..50eb05e 100644
--- a/compliance_checker/cf/cf_1_7.py
+++ b/compliance_checker/cf/cf_1_7.py
@@ -283,12 +283,7 @@ class CF1_7Check(CF1_6Check):
             ):
                 valid = False
                 reasoning.append(
-                    "Dimension {} of boundary variable (for {}) must have at least {} elements to form a simplex/closed cell with previous dimensions {}.".format(
-                        boundary_variable.name,
-                        variable.name,
-                        len(variable.dimensions) + 1,
-                        boundary_variable.dimensions[:-1],
-                    ),
+                    f"Dimension {boundary_variable.name} of boundary variable (for {variable.name}) must have at least {len(variable.dimensions) + 1} elements to form a simplex/closed cell with previous dimensions {boundary_variable.dimensions[:-1]}.",
                 )
 
             # 7.1 Required 3/5:
diff --git a/compliance_checker/cf/util.py b/compliance_checker/cf/util.py
index 35734ac..64a21c1 100644
--- a/compliance_checker/cf/util.py
+++ b/compliance_checker/cf/util.py
@@ -1,3 +1,4 @@
+import importlib.resources
 import itertools
 import os
 import sys
@@ -7,7 +8,6 @@ import requests
 from cf_units import Unit
 from lxml import etree
 from netCDF4 import Dataset
-from pkg_resources import resource_filename
 
 # copied from paegan
 # paegan may depend on these later
@@ -284,9 +284,9 @@ def download_cf_standard_name_table(version, location=None):
     if (
         location is None
     ):  # This case occurs when updating the packaged version from command line
-        location = resource_filename(
-            "compliance_checker",
-            "data/cf-standard-name-table.xml",
+        location = (
+            importlib.resources.files("compliance_checker")
+            / "data/cf-standard-name-table.xml"
         )
 
     if version == "latest":
diff --git a/compliance_checker/cfutil.py b/compliance_checker/cfutil.py
index b351a94..22195a2 100644
--- a/compliance_checker/cfutil.py
+++ b/compliance_checker/cfutil.py
@@ -3,13 +3,13 @@
 compliance_checker/cfutil.py
 """
 import csv
+import importlib.resources
 import re
 import warnings
 from collections import defaultdict
 from functools import lru_cache, partial
 
 from cf_units import Unit
-from pkg_resources import resource_filename
 
 _UNITLESS_DB = None
 _SEA_NAMES = None
@@ -128,7 +128,7 @@ def get_sea_names():
     if _SEA_NAMES is None:
         buf = {}
         with open(
-            resource_filename("compliance_checker", "data/seanames.csv"),
+            importlib.resources.files("compliance_checker") / "data/seanames.csv",
         ) as f:
             reader = csv.reader(f)
             for code, sea_name in reader:
diff --git a/compliance_checker/protocols/netcdf.py b/compliance_checker/protocols/netcdf.py
index 415a94e..4943ac8 100644
--- a/compliance_checker/protocols/netcdf.py
+++ b/compliance_checker/protocols/netcdf.py
@@ -17,6 +17,7 @@ def is_netcdf(url):
     :param str url: Location of file on the file system
     """
     # Try an obvious exclusion of remote resources
+    url = str(url)
     if url.startswith("http"):
         return False
 
diff --git a/compliance_checker/suite.py b/compliance_checker/suite.py
index d0e8476..7d92211 100644
--- a/compliance_checker/suite.py
+++ b/compliance_checker/suite.py
@@ -13,7 +13,6 @@ import textwrap
 import warnings
 from collections import defaultdict
 from datetime import datetime, timezone
-from distutils.version import StrictVersion
 from operator import itemgetter
 from pathlib import Path
 from urllib.parse import urlparse
@@ -23,6 +22,7 @@ from lxml import etree as ET
 from netCDF4 import Dataset
 from owslib.sos import SensorObservationService
 from owslib.swe.sensor.sml import SensorML
+from packaging.version import parse
 from pkg_resources import working_set
 
 from compliance_checker import __version__, tempnc
@@ -186,9 +186,8 @@ class CheckSuite:
         for spec, versions in itertools.groupby(ver_checkers, itemgetter(0)):
             version_nums = [v[-1] for v in versions]
             try:
-                latest_version = str(max(StrictVersion(v) for v in version_nums))
-            # if the version can't be parsed as a StrictVersion, parse
-            # according to character collation
+                latest_version = str(max(parse(v) for v in version_nums))
+            # if the version can't be parsed, do it according to character collation
             except ValueError:
                 latest_version = max(version_nums)
             cls.checkers[spec] = cls.checkers[spec + ":latest"] = cls.checkers[
@@ -765,12 +764,9 @@ class CheckSuite:
 
         :param str cdl_path: Absolute path to cdl file that is used to generate netCDF file
         """
-        if (
-            ".cdl" in cdl_path
-        ):  # it's possible the filename doesn't have the .cdl extension
-            ds_str = cdl_path.replace(".cdl", ".nc")
-        else:
-            ds_str = cdl_path + ".nc"
+        if isinstance(cdl_path, str):
+            cdl_path = Path(cdl_path)
+        ds_str = cdl_path.with_suffix(".nc")
 
         # generate netCDF-4 file
         iostat = subprocess.run(
diff --git a/pyproject.toml b/pyproject.toml
index af8b581..be14e45 100644
--- a/pyproject.toml
+++ b/pyproject.toml
@@ -7,7 +7,7 @@ requires = [
 ]
 
 [tool.ruff]
-select = [
+lint.select = [
     "A", # flake8-builtins
     "B", # flake8-bugbear
     "C4", # flake8-comprehensions
@@ -24,11 +24,11 @@ exclude = [
   "compliance_checker/cf/cf.py",
 ]
 
-ignore = [
+lint.ignore = [
   "E501",
 ]
 
-[tool.ruff.per-file-ignores]
+[tool.ruff.lint.per-file-ignores]
 "docs/source/conf.py" = [
   "E402",
   "A001",
diff --git a/requirements.txt b/requirements.txt
index 3df7b0d..5c04ef3 100644
--- a/requirements.txt
+++ b/requirements.txt
@@ -5,6 +5,7 @@ jinja2>=2.7.3
 lxml>=3.2.1
 netcdf4>=1.6.4
 owsLib>=0.8.3
+packaging
 pendulum>=1.2.4
 pygeoif>=0.6
 pyproj>=2.2.1


Test patch:
diff --git a/compliance_checker/tests/__init__.py b/compliance_checker/tests/__init__.py
index d8060c2..1d90751 100644
--- a/compliance_checker/tests/__init__.py
+++ b/compliance_checker/tests/__init__.py
@@ -1,4 +1,5 @@
 import unittest
+from pathlib import Path
 
 from netCDF4 import Dataset
 
@@ -25,8 +26,8 @@ class BaseTestCase(unittest.TestCase):
         """
         Return a loaded NC Dataset for the given path
         """
-        if not isinstance(nc_dataset, str):
-            raise ValueError("nc_dataset should be a string")
+        if not isinstance(nc_dataset, (str, Path)):
+            raise ValueError("nc_dataset should be a valid path")
 
         nc_dataset = Dataset(nc_dataset, "r")
         self.addCleanup(nc_dataset.close)
diff --git a/compliance_checker/tests/conftest.py b/compliance_checker/tests/conftest.py
index 482bd81..f4002bc 100644
--- a/compliance_checker/tests/conftest.py
+++ b/compliance_checker/tests/conftest.py
@@ -1,11 +1,10 @@
+import importlib.resources
 import os
 import subprocess
 from itertools import chain
-from pathlib import Path
 
 import pytest
 from netCDF4 import Dataset
-from pkg_resources import resource_filename
 
 from compliance_checker.cf import util
 from compliance_checker.suite import CheckSuite
@@ -27,7 +26,9 @@ def static_files(cdl_stem):
     Returns the Path to a valid nc dataset\n
     replaces the old STATIC_FILES dict
     """
-    datadir = Path(resource_filename("compliance_checker", "tests/data")).resolve()
+    datadir = (
+        importlib.resources.files("compliance_checker").joinpath("tests/data").resolve()
+    )
     assert datadir.exists(), f"{datadir} not found"
 
     cdl_paths = glob_down(datadir, f"{cdl_stem}.cdl", 3)
diff --git a/compliance_checker/tests/resources.py b/compliance_checker/tests/resources.py
index 4bcf314..6778617 100644
--- a/compliance_checker/tests/resources.py
+++ b/compliance_checker/tests/resources.py
@@ -1,16 +1,14 @@
-import os
+import importlib.resources
 import subprocess
 
-from pkg_resources import resource_filename
-
 
 def get_filename(path):
     """
     Returns the path to a valid dataset
     """
-    filename = resource_filename("compliance_checker", path)
-    nc_path = filename.replace(".cdl", ".nc")
-    if not os.path.exists(nc_path):
+    filename = importlib.resources.files("compliance_checker") / path
+    nc_path = filename.with_suffix(".nc")
+    if not nc_path.exists():
         generate_dataset(filename, nc_path)
     return nc_path
 
diff --git a/compliance_checker/tests/test_cf_integration.py b/compliance_checker/tests/test_cf_integration.py
index 5d16267..33f8fa2 100644
--- a/compliance_checker/tests/test_cf_integration.py
+++ b/compliance_checker/tests/test_cf_integration.py
@@ -45,9 +45,7 @@ dataset_stem__expected_messages = [
             '§2.6.1 Conventions global attribute does not contain "CF-1.8"',
             f"standard_name visibility is not defined in Standard Name Table v{std_names._version}. Possible close match(es): ['visibility_in_air']",
             'Standard name modifier "data_quality" for variable visibility_qc is not a valid modifier according to CF Appendix C',
-            "standard_name wind_direction is not defined in Standard Name Table v{}. Possible close match(es): ['wind_to_direction', 'wind_from_direction', 'wind_gust_from_direction']".format(
-                std_names._version,
-            ),
+            f"standard_name wind_direction is not defined in Standard Name Table v{std_names._version}. Possible close match(es): ['wind_to_direction', 'wind_from_direction', 'wind_gust_from_direction']",
             'Standard name modifier "data_quality" for variable wind_direction_qc is not a valid modifier according to CF Appendix C',
             f"standard_name wind_gust is not defined in Standard Name Table v{std_names._version}. Possible close match(es): ['y_wind_gust', 'x_wind_gust', 'wind_speed_of_gust']",
             'Standard name modifier "data_quality" for variable wind_gust_qc is not a valid modifier according to CF Appendix C',
diff --git a/compliance_checker/tests/test_suite.py b/compliance_checker/tests/test_suite.py
index 54f49a9..0bbbc44 100644
--- a/compliance_checker/tests/test_suite.py
+++ b/compliance_checker/tests/test_suite.py
@@ -1,35 +1,30 @@
+import importlib.resources
 import os
 import unittest
 from pathlib import Path
 
 import numpy as np
-from pkg_resources import resource_filename
 
 from compliance_checker.acdd import ACDDBaseCheck
 from compliance_checker.base import BaseCheck, GenericFile, Result
 from compliance_checker.suite import CheckSuite
 
 static_files = {
-    "2dim": resource_filename("compliance_checker", "tests/data/2dim-grid.nc"),
-    "bad_region": resource_filename("compliance_checker", "tests/data/bad_region.nc"),
-    "bad_data_type": resource_filename(
-        "compliance_checker",
-        "tests/data/bad_data_type.nc",
-    ),
-    "test_cdl": resource_filename("compliance_checker", "tests/data/test_cdl.cdl"),
-    "test_cdl_nc": resource_filename(
-        "compliance_checker",
-        "tests/data/test_cdl_nc_file.nc",
-    ),
-    "empty": resource_filename("compliance_checker", "tests/data/non-comp/empty.file"),
-    "ru07": resource_filename(
-        "compliance_checker",
-        "tests/data/ru07-20130824T170228_rt0.nc",
-    ),
-    "netCDF4": resource_filename(
-        "compliance_checker",
-        "tests/data/test_cdl_nc4_file.cdl",
-    ),
+    "2dim": importlib.resources.files("compliance_checker") / "tests/data/2dim-grid.nc",
+    "bad_region": importlib.resources.files("compliance_checker")
+    / "tests/data/bad_region.nc",
+    "bad_data_type": importlib.resources.files("compliance_checker")
+    / "tests/data/bad_data_type.nc",
+    "test_cdl": importlib.resources.files("compliance_checker")
+    / "tests/data/test_cdl.cdl",
+    "test_cdl_nc": importlib.resources.files("compliance_checker")
+    / "tests/data/test_cdl_nc_file.nc",
+    "empty": importlib.resources.files("compliance_checker")
+    / "tests/data/non-comp/empty.file",
+    "ru07": importlib.resources.files("compliance_checker")
+    / "tests/data/ru07-20130824T170228_rt0.nc",
+    "netCDF4": importlib.resources.files("compliance_checker")
+    / "tests/data/test_cdl_nc4_file.cdl",
 }
 
 
@@ -95,9 +90,9 @@ class TestSuite(unittest.TestCase):
         # create netCDF4 file
         ds_name = self.cs.generate_dataset(static_files["netCDF4"])
         # check if correct name is return
-        assert ds_name == static_files["netCDF4"].replace(".cdl", ".nc")
+        assert ds_name == static_files["netCDF4"].with_suffix(".nc")
         # check if netCDF4 file was created
-        assert os.path.isfile(static_files["netCDF4"].replace(".cdl", ".nc"))
+        assert os.path.isfile(static_files["netCDF4"].with_suffix(".nc"))
 
     def test_include_checks(self):
         ds = self.cs.load_dataset(static_files["bad_data_type"])
@@ -242,7 +237,7 @@ class TestSuite(unittest.TestCase):
             )
         ds.close()
 
-        nc_file_path = static_files["test_cdl"].replace(".cdl", ".nc")
+        nc_file_path = static_files["test_cdl"].with_suffix(".nc")
         self.addCleanup(os.remove, nc_file_path)
 
         # Ok the scores should be equal!