Skip to content
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

Ignore singleton coordinates without dims when attempting to generate bounds #281

Merged
merged 3 commits into from
Jul 26, 2022

Conversation

pochedls
Copy link
Collaborator

Description

This fixes an issue for datasets that have a singleton coordinate value (e.g., height = 2m), but not a corresponding dimension. When xcdat tries to add bounds, it hits an error, because the dimension is singleton.

Checklist

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • My changes generate no new warnings
  • Any dependent changes have been merged and published in downstream modules

If applicable:

  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass with my changes (locally and CI/CD build)
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have noted that this is a breaking change for a major release (fix or feature that would cause existing functionality to not work as expected)

@codecov-commenter
Copy link

codecov-commenter commented Jul 26, 2022

Codecov Report

Merging #281 (8a55bf5) into main (1fdc8a9) will not change coverage.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              main      #281    +/-   ##
==========================================
  Coverage   100.00%   100.00%            
==========================================
  Files            9        14     +5     
  Lines          742      1143   +401     
==========================================
+ Hits           742      1143   +401     
Impacted Files Coverage Δ
xcdat/utils.py 100.00% <ø> (ø)
xcdat/__init__.py 100.00% <100.00%> (ø)
xcdat/axis.py 100.00% <100.00%> (ø)
xcdat/bounds.py 100.00% <100.00%> (ø)
xcdat/dataset.py 100.00% <100.00%> (ø)
xcdat/regridder/__init__.py 100.00% <100.00%> (ø)
xcdat/regridder/accessor.py 100.00% <100.00%> (ø)
xcdat/regridder/base.py 100.00% <100.00%> (ø)
xcdat/regridder/grid.py 100.00% <100.00%> (ø)
xcdat/regridder/regrid2.py 100.00% <100.00%> (ø)
... and 4 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ee5b703...8a55bf5. Read the comment docs.

Comment on lines +143 to +148
# determine if the axis is also a dimension by determining
# if there is overlap between the CF axis names and the dimension
# names. If not, skip over axis for validation.
if len(set(CF_NAME_MAP[axis]) & set(self._dataset.dims.keys())) == 0:
continue

Copy link
Collaborator Author

@pochedls pochedls Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just to make it clear what is going on...

For an axis like lat, set(CF_NAME_MAP[axis]) yields {"Y", "latitude", "lat"}.

set(self._dataset.dims.keys())) yields {'axis_nbounds', 'lat', 'lon', 'time'} .

set(...) & set(...) determines the intersection between these two groups. If there are elements in the resulting set (i.e., len(...) > 0, it means that the axis is also a dimension. If there are no elements in the set, then the axis is not a dimension.

If the axis is not a dimension, there is no need to find bounds and we skip adding bounds.

Copy link
Collaborator

@tomvothecoder tomvothecoder left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me. Thanks for the quick turnaround Steve!

@tomvothecoder tomvothecoder added the type: bug Inconsistencies or issues which will cause an issue or problem for users or implementors. label Jul 26, 2022
@pochedls pochedls merged commit 6864314 into main Jul 26, 2022
@pochedls pochedls deleted the bugfix/278-cannot-generate-bounds branch July 26, 2022 20:58
tomvothecoder pushed a commit that referenced this pull request Jul 28, 2022
add unit test

add comments for test, cleanup extraneous code

initial work on #282

Bugfix/278 cannot generate bounds (#281)

* initial solution for #278

* add unit test

* add comments for test, cleanup extraneous code
pochedls added a commit that referenced this pull request Aug 2, 2022
* initial solution for #278

add unit test

add comments for test, cleanup extraneous code

initial work on #282

Bugfix/278 cannot generate bounds (#281)

* initial solution for #278

* add unit test

* add comments for test, cleanup extraneous code

* PR review refactor
- Add `types-python-dateutil` to `mypy` dependencies
- Update `ref_date` var to `ref_dt_obj` to avoid mypy error `error: Unsupported operand types for + ("str" and "relativedelta")`
- Use dictionary unpacking for units variable
- Use `datetime.strptime` instead of `pd.datetime()` which runs into the `pd.Timestamp` limitation
- Add logger.warning when non-CF compliant time coords cannot be decoded

* Consider calendar type when decoding non-Cf time
- Fix tests

* Update xcdat/dataset.py

Co-authored-by: pochedls <pochedley@gmail.com>

* Update xcdat/dataset.py

Co-authored-by: pochedls <pochedley@gmail.com>

* Fix datetime reference

* return more specific cftime datetime types

* removing extraneous calendar specification

* Refactor using xarray methods
- Move try and except statements into `decode_non_cf_time()`
- Extract function `_get_cftime_coords()` to encapsulate related logic from `decode_non_cf_time()`

* Update docstrings and silence logger warnings in tests

* Fix Timestamp limitation from dtype
- Update logger warning

* Add space in logger warning

Co-authored-by: tomvothecoder <tomvothecoder@gmail.com>
@tomvothecoder tomvothecoder changed the title Bugfix/278 cannot generate bounds Ignore singleton coordinates when attempting to generate bounds upon opening dataset. Aug 17, 2022
@tomvothecoder tomvothecoder changed the title Ignore singleton coordinates when attempting to generate bounds upon opening dataset. Ignore singleton coordinates when attempting to generate bounds upon opening dataset Aug 17, 2022
@tomvothecoder tomvothecoder changed the title Ignore singleton coordinates when attempting to generate bounds upon opening dataset Ignore singleton coordinates without dims when attempting to generate bounds Aug 17, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug Inconsistencies or issues which will cause an issue or problem for users or implementors.
Projects
No open projects
Status: Done
3 participants