Skip to content

Use keyword-only arguments in neurodata_type constructors #1189

@rly

Description

@rly

Problem:
For the spec of a given neurodata_type, if we make a required group/dataset/attribute/link optional, then in the PyNWB API, the only way to make the field optional in the constructor is to add 'default': None to the docval of the field. If the field has a required field following it in the docval list of fields, and if the user provides positional arguments, then the parsing of arguments by docval will be incorrect. This has prevented us from making such changes as making NWBFile.identifier optional because it would break all existing code that uses positional arguments.

Proposed solution:
I propose we do not allow positional arguments for __init__ of all neurodata_types. This has the added benefit of making user code more readable, albeit a little bulkier. Users would have to consult documentation to get the argument names, but in my experience, most users do this already because no one has memorized the many possible arguments and their order for each class. The order is also inconsistent across classes.

This would break any existing code that uses positional arguments and so should probably wait for the next major release.

In regard to the autodoc, we can use the syntax of keyword-only arguments that was introduced in Python 3: https://www.python.org/dev/peps/pep-3102/ . Basically, place a * in the arguments list before the keyword-only arguments, e.g., the function signature in the docs would appear as:

class A:
    def __init__(self, *, arg1=None, arg2=None):
        print(kwargs)

And we should add an explicit note saying positional arguments are not allowed in this method.

In order to transition to this change in the next major version, I propose we show warnings when positional arguments are passed to neurodata_type constructors, and update our docs and tests to use only keyword arguments.

Now:

  • warn when positional arguments are passed to neurodata_type constructors
  • update tests
  • update docs

Later:

  • change warning to error

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions