Skip to content

Aggregation parameters: mismatch between actual & doc'd #115

@sadielbartholomew

Description

@sadielbartholomew

There are two discrepancies between the documented parameters of the cf.aggregate function and the actual parameters it accepts, namely:

  • cf.aggregates takes the undocumented parameters, with defaults, ignore=None & exclude=False;

    cf-python/cf/aggregate.py

    Lines 1293 to 1294 in db0a9f8

    ignore=None,
    exclude=False,

  • the corresponding API reference describes undefined parameters exist_ignore and equal_ignore (with no specific defaults indicated in the summary text):

    >>> a = cf.example_field(0)
    >>> b = cf.aggregate(a, equal_ignore=False)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/sadie/cfdm/cfdm/decorators.py", line 186, in verbose_override_wrapper
        return method_with_verbose_kwarg(self, *args, **kwargs)
    TypeError: aggregate() got an unexpected keyword argument 'equal_ignore'
    >>> b = cf.aggregate(a, exist_ignore=False)
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
      File "/home/sadie/cfdm/cfdm/decorators.py", line 186, in verbose_override_wrapper
        return method_with_verbose_kwarg(self, *args, **kwargs)
    TypeError: aggregate() got an unexpected keyword argument 'exist_ignore'

(I double-checked and found those are the only mismatches.)

It seems most likely given the equal number of parameters missing in each case that the pair of undocumented parameters were intended as the pair of documented but undefined parameters, but both having been given the wrong name in the API reference. I just want to check that this is indeed the case, and whether the mapping is (as there is not much in the logic to indicate potential matches & from the dev history it seems the mismatch was there as far back as at least 2015):

  • ignore -> equal_ignore;
  • exclude -> exist_ignore;

or vice versa. Arrows indicate the direction of change, as well, since to avoid an API change it seems preferable to re-document the mapping than to make an equivalent re-name of the parameters, do you agree @davidhassell ?

While we are considering it, I feel these names could be difficult to distinguish from each other in function without having to look-up the definitions, so perhaps we could rename them to try to make them more distinctive? Or do your recall naming these parameters was difficult such that these names might be the best possible?

Metadata

Metadata

Assignees

No one assigned

    Labels

    aggregationRerlating to metadata-based field and domain aggregationquestionGeneral question

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions