Skip to content

Conversation

@sadielbartholomew
Copy link
Member

@sadielbartholomew sadielbartholomew commented Aug 2, 2022

Migrating the Data.concatenate_data method towards #182, by deprecating it.

I don't think this approach will be controversial (so-as to have required pre-PR discussion) since it seems to be essentially the same method as concatenate, just compare the inputs and outputs on the docstrings to see this, with no real difference except for the behaviour of:

In the case that the list contains only one element, that element is simply returned.

and it is not used anywhere throughout the codebase, unlike concatenate which is used quite widely (note this was a post-PR grep, but regardless the only usage is to define it):

$ pwd
/home/sadie/cf-python/cf
$ git grep "concatenate_data"
data/mixin/deprecations.py:    def concatenate_data(cls, data_list, axis):
data/mixin/deprecations.py:            "Data method 'concatenate_data' has been deprecated at "

Anyhow @davidhassell you can object at this stage and close if there is a justification to keep the method, else review this towards merging, as the change itself is almost trivial. Thanks.

Copy link
Collaborator

@davidhassell davidhassell left a comment

Choose a reason for hiding this comment

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

Great.

@sadielbartholomew sadielbartholomew merged commit fb7cc5f into NCAS-CMS:lama-to-dask Aug 2, 2022
@sadielbartholomew sadielbartholomew deleted the lama-to-dask-concat-data branch August 2, 2022 07:03
@davidhassell davidhassell added this to the 3.14.0 milestone Nov 15, 2022
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants