Skip to content

Make most keyword parameters invalid as positional arguments  #1130

@rmjarvis

Description

@rmjarvis

Now that we don't have to support Python 2.7 anymore, we can start using the * in function signatures to indicate keyword-only arguments. IMO, this tends to make user code better, forcing them to self-document what things mean by naming parameters, rather than let them be cryptically just False or 3.5, etc.

In general, I think we should make almost everything keyword-only. The main exceptions are for the first argument of a function, when the meaning of that argument is clear from the name of the function. e.g. image.addNoise(noise) or obj.setFlux(flux). Occasionally there are cases in GalSim code where both of the first two arguments are fairly obvious from context. e.g. obj.shift(x,y).

The other main exception will be for our "leading underscore" functions. I don't know what the latest performance difference is between args and kwargs, but it at least used to be the case that kwarg parsing was slower (cf. https://bugs.python.org/issue27574). This makes some sense, since it requires some string parsing rather than just counting items in a list. I don't know how much of a difference it makes in practice, but since our leading underscore functions are designed to whittle away as much overhead as possible for performance, let's not add any back by making them use kwargs.

Beyond these kinds of cases, I think we should usually favor forcing the user to explicitly name any parameters they are using. But there may be some other exceptions here and there where it might make sense to allow a couple parameters to remain positional. I suspect we can probably use our own code (tests, examples, and internal calls) as a guide for when to allow positional arguments. If we found it clear enough to write something without a keyword, we might probably want to let users do so too.

This is technically an API change, but I have a decorator we can use during the 2.x series to make it warn for violations rather than raise TypeErrors, which I wrote for the corresponding effort for TreeCorr. cf. https://github.com/rmjarvis/TreeCorr/pull/129/files#diff-56ac2f79094c0869fb8bbe6744b6cbcd7e270e9f6cac90b9373993ec2c14524fR927

Metadata

Metadata

Assignees

No one assigned

    Labels

    cleanupNon-functional changes to make the code better

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions