Skip to content

Conversation

davidhassell
Copy link
Collaborator

  • Add missing Container mixin class inheritance to array classes. This is needed to pick up the cf docstring substitutions.
  • Allow no-arg init of CFANetCDFArray

Necessary (but not sufficient) to get test_docstring.py to pass.

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.

The first point (from your opening comment) has been achieved, but the second point promise of:

Allow no-arg init of CFANetCDFArray

seems to require some tweaks since I see:

>>> import cf
>>> cf.CFANetCDFArray()
Traceback (most recent call last):
  File "/home/sadie/cfdm/cfdm/core/abstract/container.py", line 259, in _get_component
    return self._components[component]
KeyError: 'shape'

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/sadie/cf-python/cf/data/array/netcdfarray.py", line 20, in __repr__
    return super().__repr__().replace("<", "<CF ", 1)
  File "/home/sadie/cfdm/cfdm/data/netcdfarray.py", line 295, in __repr__
    return f"<{self.__class__.__name__}{self.shape}: {self}>"
  File "/home/sadie/cfdm/cfdm/data/netcdfarray.py", line 409, in shape
    return self._get_component("shape")
  File "/home/sadie/cfdm/cfdm/core/abstract/container.py", line 264, in _get_component
    return self._default(
  File "/home/sadie/cfdm/cfdm/core/abstract/container.py", line 141, in _default
    raise default
ValueError: CFANetCDFArray has no 'shape' component

Otherwise everything looks good!

@davidhassell
Copy link
Collaborator Author

Hi Sadie - I'm happy to let this go, as the no-arg init is working fine ... it's just the repr of it that fails, and that's OK :)

@davidhassell
Copy link
Collaborator Author

Hi Sadie - couldn't stop thinking about this. You're right that repr should also work, but as this class is a placeholder for 4.0.0 and is not used at all by 3.14.0, it's probably best to not develop if further until after 3.14.0, as we'll probably only change what we might do now. Thanks.

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 the clarification regarding the repr. I'm happy given your justification that it doesn't work right now.

Feedback/questions all addressed. Great, please merge.

@davidhassell davidhassell merged commit ec53043 into NCAS-CMS:lama-to-dask Jan 30, 2023
@davidhassell davidhassell deleted the dask-container branch February 6, 2023 09:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dask Relating to the use of Dask high priority
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants