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

LAMA to Dask: (deprecate) Data.concatenate_data #426

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