re-order `add_feature` arguments

[DaniBodor]

The function is currently defined by default as follows:

def add_features(
    pdb_path: str,  # noqa: ARG001
    graph: Graph,
    single_amino_acid_variant: SingleResidueVariant | None = None,  # noqa: ARG001
) -> None:

There is only 1 feature for which the pdb_path is actually used. However as currently implemented it must always be given. For this reason, it would make more sense if pdb_path is requested last, so that it can just be omitted in most add_features calls.

But I think it would be even better to always take **kwargs as an argument in this function. Any specific argument needed for the feature in question can be explicitly named, and all others can be "hidden" in the kwargs. This also opens up the possibility for adding other keywords to future (or user defined) features that are not currently used, while maintaining a default structure.

[DaniBodor]

I wonder whether this would be considered a breaking change (requiring a new major version). On the one hand, calling the add_features function is done behind the scenes by the API. However, for any user defined features, this could now break the code.

[gcroci2]

Good points, thanks for raising the issue!

I wonder whether this would be considered a breaking change (requiring a new major version). On the one hand, calling the add_features function is done behind the scenes by the API. However, for any user defined features, this could now break the code.

Indeed, this will need a major release.

[github-actions[bot]]

This issue is stale because it has been open for 30 days with no activity.