Repository: xCDAT/xcdat

Issue:
[Bug]: decode_times=True as default triggers error when open netcdf with no time dimension
### What happened?

Opened 2-d field (lat/lon) written in netcdf file using xcdat. Error triggered if without `decode_times=False` option given.

![Screen Shot 2022-12-09 at 4 26 57 PM](https://user-images.githubusercontent.com/17091564/206818736-a53840e1-45b5-4b9a-96ed-c44b0f75da01.png)


### What did you expect to happen?

Because the dataset does not have time dimension and has only lat/lon dimensions, I was expecting `decode_times` won't be activated even if it is a default option.

### Minimal Complete Verifiable Example

```python
import xcdat as xc
target_grid = xc.open_dataset('target_grid.nc')
```


### Relevant log output

```python
---------------------------------------------------------------------------
KeyError                                  Traceback (most recent call last)
Cell In[30], line 2
      1 import xcdat as xc
----> 2 target_grid = xc.open_dataset('target_grid.nc')

File ~/.conda/envs/pcmdi_metrics_dev_12_20221208/lib/python3.9/site-packages/xcdat/dataset.py:103, in open_dataset(path, data_var, add_bounds, decode_times, center_times, lon_orient, **kwargs)
    100 ds = xr.open_dataset(path, decode_times=False, **kwargs)  # type: ignore
    102 if decode_times:
--> 103     ds = decode_time(ds)
    105 ds = _postprocess_dataset(ds, data_var, center_times, add_bounds, lon_orient)
    107 return ds

File ~/.conda/envs/pcmdi_metrics_dev_12_20221208/lib/python3.9/site-packages/xcdat/dataset.py:308, in decode_time(dataset)
    305 coord_keys = _get_all_coord_keys(ds, "T")
    307 if len(coord_keys) == 0:
--> 308     raise KeyError(
    309         "Unable to map to time coordinates in this dataset to perform decoding. "
    310         "Make sure that the time coordinates have the CF 'axis' or 'standard_name' "
    311         "attribute set (e.g., ds['time'].attrs['axis'] = 'T' or "
    312         "ds['time'].attrs['standard_name'] = 'time'), and try decoding again. "
    313     )
    315 for key in coord_keys:
    316     coords = ds[key].copy()

KeyError: "Unable to map to time coordinates in this dataset to perform decoding. Make sure that the time coordinates have the CF 'axis' or 'standard_name' attribute set (e.g., ds['time'].attrs['axis'] = 'T' or ds['time'].attrs['standard_name'] = 'time'), and try decoding again. "
```


### Anything else we need to know?

Example input included here (uncompress to use):
[target_grid.nc.zip](https://github.com/xCDAT/xcdat/files/10198872/target_grid.nc.zip)


### Environment

xcdat 0.4.0

Discussion:
tomvothecoder: Hey @lee1043 and @pochedls, this PR is ready for review. 

I tested it locally using dummy datasets without time coordinates and it raises the warning as expected. It would be good to test using the dataset from the example in #396 too.
codecov-commenter: # [Codecov](https://codecov.io/gh/xCDAT/xcdat/pull/409?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT) Report
Base: **100.00**% // Head: **100.00**% // No change to project coverage :thumbsup:
> Coverage data is based on head [(`adcb28d`)](https://codecov.io/gh/xCDAT/xcdat/pull/409?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT) compared to base [(`1a0afb4`)](https://codecov.io/gh/xCDAT/xcdat/commit/1a0afb4a299665d62d8944c77e6819dee3e055e0?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT).
> Patch coverage: 100.00% of modified lines in pull request are covered.

<details><summary>Additional details and impacted files</summary>


```diff
@@            Coverage Diff            @@
##              main      #409   +/-   ##
=========================================
  Coverage   100.00%   100.00%           
=========================================
  Files           14        14           
  Lines         1254      1260    +6     
=========================================
+ Hits          1254      1260    +6     
```


| [Impacted Files](https://codecov.io/gh/xCDAT/xcdat/pull/409?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT) | Coverage Δ | |
|---|---|---|
| [xcdat/dataset.py](https://codecov.io/gh/xCDAT/xcdat/pull/409?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT#diff-eGNkYXQvZGF0YXNldC5weQ==) | `100.00% <100.00%> (ø)` | |

Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT)

</details>

[:umbrella: View full report at Codecov](https://codecov.io/gh/xCDAT/xcdat/pull/409?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT).   
:loudspeaker: Do you have feedback about the report comment? [Let us know in this issue](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=xCDAT).

lee1043: > I tested it locally using dummy datasets without time coordinates and it raises the warning as expected. It would be good to test using the dataset from the example in #396 too.

@tomvothecoder below is what was happening when I open the example data in #396 from terminal. 

```
Python 3.10.6 | packaged by conda-forge | (main, Aug 22 2022, 20:41:54) [Clang 13.0.1 ] on darwin
Type "help", "copyright", "credits" or "license" for more information.
>>> import xcdat as xc
>>> path = 'target_grid.nc'
>>> ds = xc.open_dataset(path)
2023-01-26 11:23:21,603 [WARNING]: dataset.py(open_dataset:106) >> "No time coordinates were found in this dataset to decode. Make sure time coordinates exist and their CF 'axis' or 'standard_name' attribute is set (e.g., ds['time'].attrs['axis'] = 'T' or ds['time'].attrs['standard_name'] = 'time'). Afterwards, try decoding with `decode_time()` again."
```

I think revising the error message slightly may be helpful for clarifying. I suggest something like below: 
No time coordinates were found in this dataset to decode. **If existence of time coordinates is expected, m**ake sure time coordinates exist and their CF 'axis' or 'standard_name' attribute is set (e.g., ds['time'].attrs['axis'] = 'T' or ds['time'].attrs['standard_name'] = 'time')
tomvothecoder: > @tomvothecoder below is what I see when I open the example data in #396 from terminal. It is great that data opened with no problem.
> 
> ```
> Python 3.10.6 | packaged by conda-forge | (main, Aug 22 2022, 20:41:54) [Clang 13.0.1 ] on darwin
> Type "help", "copyright", "credits" or "license" for more information.
> >>> import xcdat as xc
> >>> path = 'target_grid.nc'
> >>> ds = xc.open_dataset(path)
> 2023-01-26 11:23:21,603 [WARNING]: dataset.py(open_dataset:106) >> "No time coordinates were found in this dataset to decode. Make sure time coordinates exist and their CF 'axis' or 'standard_name' attribute is set (e.g., ds['time'].attrs['axis'] = 'T' or ds['time'].attrs['standard_name'] = 'time'). Afterwards, try decoding with `decode_time()` again."
> ```
> 
> I understand @pochedls prefers error message to not shown in console and I agree. 

Thank you for your input Jiwoo. @pochedls is actually in [agreement](https://github.com/xCDAT/xcdat/pull/409#discussion_r1087432563) with my revision to raise a warning instead of silencing it. A warning is raised if time coordinates cannot be detected with `decode_times=True` (the default). 
Time coordinates **can't be detected if they don't exist** or **don't have the appropriate CF attributes** to map to them.

 If we expect `decode_times=True` to work in most cases, I think it is reasonable to raise a warning since it won't appear often and they can override `decode_times=False` if they want.  Also, it is probably not good idea to silence potential warnings or errors because the user might expect time coordinates to be decoded/decodeable but that isn't true (and they find out later in their temporal workflow such as averaging).

> In parallel, I think revising the error message slightly may be helpful for clarifying. I suggest something like below: No time coordinates were found in this dataset to decode. **If existence of time coordinates is expected, m**ake sure time coordinates exist and their CF 'axis' or 'standard_name' attribute is set (e.g., ds['time'].attrs['axis'] = 'T' or ds['time'].attrs['standard_name'] = 'time')

Good suggestion. I agree with revising the warning message to make it clearer. 
tomvothecoder: Merging this PR

Patch:
diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml
index 1ab4c4a..72cbbcd 100644
--- a/.pre-commit-config.yaml
+++ b/.pre-commit-config.yaml
@@ -16,7 +16,7 @@ repos:
           - id: black
 
     - repo: https://github.com/timothycrosley/isort
-      rev: 5.11.4
+      rev: 5.12.0
       hooks:
           - id: isort
 
diff --git a/conda-env/dev.yml b/conda-env/dev.yml
index 0321953..f1eefed 100644
--- a/conda-env/dev.yml
+++ b/conda-env/dev.yml
@@ -41,7 +41,7 @@ dependencies:
     - black=22.12.0
     - flake8=6.0.0
     - flake8-isort=6.0.0
-    - isort=5.11.4
+    - isort=5.12.0
     - mypy=0.991
     - pre-commit=2.21.0
     - types-python-dateutil=2.8.19
diff --git a/xcdat/dataset.py b/xcdat/dataset.py
index 6fc0ed6..5969b12 100644
--- a/xcdat/dataset.py
+++ b/xcdat/dataset.py
@@ -100,7 +100,10 @@ def open_dataset(
     ds = xr.open_dataset(path, decode_times=False, **kwargs)  # type: ignore
 
     if decode_times:
-        ds = decode_time(ds)
+        try:
+            ds = decode_time(ds)
+        except KeyError as err:
+            logger.warning(err)
 
     ds = _postprocess_dataset(ds, data_var, center_times, add_bounds, lon_orient)
 
@@ -243,6 +246,13 @@ def decode_time(dataset: xr.Dataset) -> xr.Dataset:
         Dataset with decoded time coordinates and time bounds (if they exist) as
         ``cftime`` objects.
 
+    Raises
+    ------
+    KeyError
+        If time coordinates were not detected in the dataset, either because they
+        don't exist at all or their CF attributes (e.g., 'axis' or
+        'standard_name') are not set.
+
     Notes
     -----
     Time coordinates are represented by ``cftime.datetime`` objects because
@@ -306,10 +316,12 @@ def decode_time(dataset: xr.Dataset) -> xr.Dataset:
 
     if len(coord_keys) == 0:
         raise KeyError(
-            "Unable to map to time coordinates in this dataset to perform decoding. "
-            "Make sure that the time coordinates have the CF 'axis' or 'standard_name' "
-            "attribute set (e.g., ds['time'].attrs['axis'] = 'T' or "
-            "ds['time'].attrs['standard_name'] = 'time'), and try decoding again. "
+            "No time coordinates were found in this dataset to decode. If time "
+            "coordinates were expected to exist, make sure they are detectable by "
+            "setting the CF 'axis' or 'standard_name' attribute (e.g., "
+            "ds['time'].attrs['axis'] = 'T' or "
+            "ds['time'].attrs['standard_name'] = 'time'). Afterwards, try decoding "
+            "again with `xcdat.decode_time`."
         )
 
     for key in coord_keys:
@@ -376,6 +388,7 @@ def _preprocess(
         The Dataset.
     callable : Optional[Callable], optional
         A user specified optional callable function for preprocessing.
+
     Returns
     -------
     xr.Dataset
@@ -387,7 +400,10 @@ def _preprocess(
         ds_new = callable(ds)
 
     if decode_times:
-        ds_new = decode_time(ds_new)
+        try:
+            ds_new = decode_time(ds_new)
+        except KeyError as err:
+            logger.warning(err)
 
     return ds_new
 


Test patch:
diff --git a/tests/test_dataset.py b/tests/test_dataset.py
index a10177e..9ce191d 100644
--- a/tests/test_dataset.py
+++ b/tests/test_dataset.py
@@ -27,6 +27,29 @@ class TestOpenDataset:
         dir.mkdir()
         self.file_path = f"{dir}/file.nc"
 
+    def test_raises_warning_if_decode_times_but_no_time_coords_found(self, caplog):
+        ds = generate_dataset(decode_times=False, cf_compliant=True, has_bounds=True)
+        ds = ds.drop_dims("time")
+        ds.to_netcdf(self.file_path)
+
+        result = open_dataset(self.file_path)
+        expected = generate_dataset(
+            decode_times=False,
+            cf_compliant=True,
+            has_bounds=True,
+        )
+        expected = expected.drop_dims("time")
+
+        assert result.identical(expected)
+        assert (
+            "No time coordinates were found in this dataset to decode. If time "
+            "coordinates were expected to exist, make sure they are detectable by "
+            "setting the CF 'axis' or 'standard_name' attribute (e.g., "
+            "ds['time'].attrs['axis'] = 'T' or "
+            "ds['time'].attrs['standard_name'] = 'time'). Afterwards, try decoding "
+            "again with `xcdat.decode_time`."
+        ) in caplog.text
+
     def test_skip_decoding_time_explicitly(self):
         ds = generate_dataset(decode_times=False, cf_compliant=True, has_bounds=True)
         ds.to_netcdf(self.file_path)
@@ -567,6 +590,29 @@ class TestOpenMfDataset:
         self.file_path1 = f"{dir}/file1.nc"
         self.file_path2 = f"{dir}/file2.nc"
 
+    def test_raises_warning_if_decode_times_but_no_time_coords_found(self, caplog):
+        ds = generate_dataset(decode_times=False, cf_compliant=True, has_bounds=True)
+        ds = ds.drop_dims("time")
+        ds.to_netcdf(self.file_path1)
+
+        result = open_mfdataset(self.file_path1)
+        expected = generate_dataset(
+            decode_times=False,
+            cf_compliant=True,
+            has_bounds=True,
+        )
+        expected = expected.drop_dims("time")
+
+        assert result.identical(expected)
+        assert (
+            "No time coordinates were found in this dataset to decode. If time "
+            "coordinates were expected to exist, make sure they are detectable by "
+            "setting the CF 'axis' or 'standard_name' attribute (e.g., "
+            "ds['time'].attrs['axis'] = 'T' or "
+            "ds['time'].attrs['standard_name'] = 'time'). "
+            "Afterwards, try decoding again with `xcdat.decode_time`."
+        ) in caplog.text
+
     def test_skip_decoding_times_explicitly(self):
         ds1 = generate_dataset(decode_times=False, cf_compliant=False, has_bounds=True)
         ds1.to_netcdf(self.file_path1)