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 cfdm.write failure for a vertical CRS that has no coordinates #165

Merged
merged 4 commits into from
Nov 3, 2021

Conversation

davidhassell
Copy link
Contributor

Fixes #164

@codecov
Copy link

codecov bot commented Nov 3, 2021

Codecov Report

Merging #165 (4cf3c08) into master (8e6ac54) will decrease coverage by 0.01%.
The diff coverage is 100.00%.

❗ Current head 4cf3c08 differs from pull request most recent head e16039d. Consider uploading reports for the commit e16039d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #165      +/-   ##
==========================================
- Coverage   88.72%   88.71%   -0.00%     
==========================================
  Files         101      101              
  Lines       10806    10806              
==========================================
- Hits         9587     9586       -1     
- Misses       1219     1220       +1     
Flag Coverage Δ
unittests 88.71% <100.00%> (-<0.01%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
cfdm/read_write/netcdf/netcdfwrite.py 86.63% <100.00%> (ø)
cfdm/read_write/netcdf/netcdfread.py 83.45% <0.00%> (-0.05%) ⬇️

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 8e6ac54...e16039d. Read the comment docs.

Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

Good fix, but can we have a test to cover the case of writing a field with a vertical coordinate reference system? Nothing fails on master to indicate the issue at hand...

@davidhassell
Copy link
Contributor Author

Hi Sadie, Fair enough! The error came from some user data, and I'll try to get a minimal reproducer we can include in the tests.

@davidhassell davidhassell changed the title Fix cfdm.write failure for a field with a vertical coordinate reference system Fix cfdm.write failure for a vertical CRS that has no coordinates Nov 3, 2021
Copy link
Member

@sadielbartholomew sadielbartholomew left a comment

Choose a reason for hiding this comment

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

Thanks for adding the new test, which passes on this PR branch but fails on master due to the issue at hand:

ERROR: test_CoordinateReference_write (__main__.CoordinateReferenceTest)
Test write when vertical CRS has no coordinates.
----------------------------------------------------------------------
Traceback (most recent call last):
  File "test_CoordinateReference.py", line 61, in test_CoordinateReference_write
    cfdm.write(f, tempfile1)
  File "/home/sadie/cfdm/cfdm/read_write/write.py", line 522, in write
    netcdf.write(
  File "/home/sadie/cfdm/cfdm/decorators.py", line 171, in verbose_override_wrapper
    return method_with_verbose_kwarg(*args, **kwargs)
  File "/home/sadie/cfdm/cfdm/read_write/netcdf/netcdfwrite.py", line 4809, in write
    self._file_io_iteration(
  File "/home/sadie/cfdm/cfdm/read_write/netcdf/netcdfwrite.py", line 5089, in _file_io_iteration
    self._write_field_or_domain(f)
  File "/home/sadie/cfdm/cfdm/read_write/netcdf/netcdfwrite.py", line 3725, in _write_field_or_domain
    z_axis = self.implementation.get_construct_data_axes(
TypeError: 'NoneType' object is not subscriptable

----------------------------------------------------------------------

so captures the bug. All good!

@sadielbartholomew sadielbartholomew merged commit 679a095 into NCAS-CMS:master Nov 3, 2021
@davidhassell
Copy link
Contributor Author

Thank you, Sadie.

@davidhassell davidhassell added this to the 1.9.0.2 milestone Jan 31, 2022
@davidhassell davidhassell deleted the write-bug-fix branch November 14, 2022 09:00
@davidhassell davidhassell added the netCDF write Relating to writing netCDF datasets label Nov 15, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
netCDF write Relating to writing netCDF datasets
Projects
None yet
Development

Successfully merging this pull request may close these issues.

cfdm.write fails for a vertical CRS with no coordinates
2 participants