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

Fix in extra_coords #238

Merged
merged 21 commits into from
Jul 30, 2021
Merged

Fix in extra_coords #238

merged 21 commits into from
Jul 30, 2021

Conversation

aurghs
Copy link
Contributor

@aurghs aurghs commented Jul 20, 2021

This PR fixes two bugs on the extra_coords keyword:

  1. Currently it is not possible to use extra_coords with the new cfgrib-xarray interface (xarray_plugin.py). I have added extra_coords in the new interface to fix it.

  2. If the user tries to read an extra coordinate with a scalar dimension, cfrgib returns an empty coordinate and xarray raise an error because the coordinates and dimensions are not consistent.

old behaviour:

res = xr.open_dataset(
        path, extra_coords={"experimentVersionNumber": "time"}
    )

---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
/var/folders/44/qn_zhjz165gflnp_1_x0p9g40000gn/T/ipykernel_89177/3004447257.py in <module>
      1 TEST_DATA_SCALAR_TIME = "/Users/barghini/devel/wp6/cfgrib/tests/sample-data/era5-single-level-scalar-time.grib"
      2 
----> 3 res = xr.open_dataset(
      4         path, extra_coords={"experimentVersionNumber": "time"}
      5     )

/usr/local/Caskroom/miniconda/base/envs/cfgrib-wp6/lib/python3.9/site-packages/xarray/backends/api.py in open_dataset(filename_or_obj, engine, chunks, cache, decode_cf, mask_and_scale, decode_times, decode_timedelta, use_cftime, concat_characters, decode_coords, drop_variables, backend_kwargs, *args, **kwargs)
    494 
    495     overwrite_encoded_chunks = kwargs.pop("overwrite_encoded_chunks", None)
--> 496     backend_ds = backend.open_dataset(
    497         filename_or_obj,
    498         drop_variables=drop_variables,

~/devel/wp6/cfgrib/cfgrib/xarray_entrypoint.py in open_dataset(self, filename_or_obj, mask_and_scale, decode_times, concat_characters, decode_coords, drop_variables, use_cftime, decode_timedelta, lock, indexpath, filter_by_keys, read_keys, encode_cf, squeeze, time_dims, errors, extra_coords)

/usr/local/Caskroom/miniconda/base/envs/cfgrib-wp6/lib/python3.9/site-packages/xarray/core/dataset.py in __init__(self, data_vars, coords, attrs)
    737             coords = coords.variables
    738 
--> 739         variables, coord_names, dims, indexes, _ = merge_data_and_coords(
    740             data_vars, coords, compat="broadcast_equals"
    741         )

/usr/local/Caskroom/miniconda/base/envs/cfgrib-wp6/lib/python3.9/site-packages/xarray/core/merge.py in merge_data_and_coords(data, coords, compat, join)
    475     explicit_coords = coords.keys()
    476     indexes = dict(_extract_indexes_from_coords(coords))
--> 477     return merge_core(
    478         objects, compat, join, explicit_coords=explicit_coords, indexes=indexes
    479     )

/usr/local/Caskroom/miniconda/base/envs/cfgrib-wp6/lib/python3.9/site-packages/xarray/core/merge.py in merge_core(objects, compat, join, combine_attrs, priority_arg, explicit_coords, indexes, fill_value)
    629     assert_unique_multiindex_level_names(variables)
    630 
--> 631     dims = calculate_dimensions(variables)
    632 
    633     coord_names, noncoord_names = determine_coords(coerced)

/usr/local/Caskroom/miniconda/base/envs/cfgrib-wp6/lib/python3.9/site-packages/xarray/core/dataset.py in calculate_dimensions(variables)
    195         for dim, size in zip(var.dims, var.shape):
    196             if dim in scalar_vars:
--> 197                 raise ValueError(
    198                     f"dimension {dim!r} already exists as a scalar variable"
    199                 )

ValueError: dimension 'time' already exists as a scalar variable

new behaviour:

res = xr.open_dataset(
        path, extra_coords={"experimentVersionNumber": "time"}
    )
print(res)

<xarray.Dataset>
Dimensions:                  (latitude: 121, longitude: 201)
Coordinates:
    number                   int64 ...
    time                     datetime64[ns] ...
    step                     timedelta64[ns] ...
    surface                  float64 ...
  * latitude                 (latitude) float64 60.0 59.75 59.5 ... 30.25 30.0
  * longitude                (longitude) float64 -10.0 -9.75 -9.5 ... 39.75 40.0
    valid_time               datetime64[ns] ...
    experimentVersionNumber  object ...
Data variables:
    t2m                      object ...
Attributes:
    GRIB_edition:            1
    GRIB_centre:             ecmf
    GRIB_centreDescription:  European Centre for Medium-Range Weather Forecasts
    GRIB_subCentre:          0
    Conventions:             CF-1.7
    institution:             European Centre for Medium-Range Weather Forecasts
    history:                 2021-07-20T11:32 GRIB to CDM+CF via cfgrib-0.9.9...

@codecov-commenter
Copy link

codecov-commenter commented Jul 20, 2021

Codecov Report

Merging #238 (faac3a3) into master (9145591) will increase coverage by 0.08%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #238      +/-   ##
==========================================
+ Coverage   92.24%   92.33%   +0.08%     
==========================================
  Files          25       25              
  Lines        1754     1774      +20     
  Branches      206      209       +3     
==========================================
+ Hits         1618     1638      +20     
  Misses        112      112              
  Partials       24       24              
Impacted Files Coverage Δ
cfgrib/xarray_plugin.py 0.00% <ø> (ø)
cfgrib/dataset.py 98.97% <100.00%> (+0.03%) ⬆️
tests/test_30_dataset.py 100.00% <100.00%> (ø)
tests/test_50_sample_data.py 100.00% <100.00%> (ø)

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 9145591...faac3a3. Read the comment docs.

@aurghs aurghs changed the title WIP: Fix in new feature extra_coords WIP: Fix in extra_coords Jul 20, 2021
@aurghs aurghs requested a review from alexamici July 20, 2021 16:58
@aurghs aurghs changed the title WIP: Fix in extra_coords Fix in extra_coords Jul 21, 2021
@aurghs aurghs requested a review from iainrussell July 21, 2021 07:28
@aurghs aurghs changed the title Fix in extra_coords WIP: Fix in extra_coords Jul 22, 2021
@aurghs aurghs changed the title WIP: Fix in extra_coords Fix in extra_coords Jul 29, 2021
Copy link
Contributor

@alexamici alexamici left a comment

Choose a reason for hiding this comment

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

I suggested a few style fixes. LGTM otherwise.

cfgrib/dataset.py Outdated Show resolved Hide resolved
cfgrib/dataset.py Show resolved Hide resolved
@alexamici
Copy link
Contributor

@aurghs this PR also addresses #231 right?

@alexamici alexamici merged commit 967001d into ecmwf:master Jul 30, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants