Refactor argument handling to enforce positional-only and keyword-only arguments using `/` and `*`

edoaltamura commented 2 weeks ago

What should we add?

Description

To improve readability and usability in the Qiskit codebase, we propose refactoring function signatures to clearly define positional-only, positional-or-keyword, and keyword-only arguments using the / and * symbols. Parameters before the slash will be strictly positional, while those after the asterisk will be keyword-only. This approach clarifies the API and aligns with Python standards, as in Scikit-learn.

We can start with function signatures and see which arguments should be made keyword-only or remain positional. Temporary lint disables may be necessary to prevent errors during the transition, as seen in #833.

Example from Pegasos QSVC

def __init__(
        self,
        quantum_kernel: BaseKernel | None = None,
        C: float = 1.0,
        num_steps: int = 1000,
        precomputed: bool = False,
        seed: int | None = None,
    ) -> None:

into

def __init__(
        self,
        quantum_kernel: BaseKernel | None = None,
        *,
        C: float = 1.0,
        num_steps: int = 1000,
        precomputed: bool = False,
        seed: int | None = None,
    ) -> None:

tsmanral commented 4 days ago

Hi @edoaltamura. Does this issue still exists ? If yes, I'd like to assign it to myself and contribute. Please let me know.

edoaltamura commented 3 days ago

Hi @tsmanral, it does! Thanks for your interest in this project. You're very welcome to scan the code for definitions that need an update. I'd like to make two suggestions:

Keep a checklist of files/code instances that need the change either in this issue or in a draft pull request - this is useful for logging purposes and would also help us give you a hand if you need it.
You can consult the guidelines for contributions in CONTRIBUTING.md. Specifically, the make lint command shortcut will be useful for this fix.

edoaltamura commented 3 days ago

Also related to linting, this old issue https://github.com/qiskit-community/qiskit-machine-learning/issues/159 might be tackled with a similar approach. Numpy typing has become more stable since the issue was first opened, see https://numpy.org/doc/stable/reference/typing.html

tsmanral commented 2 days ago

Thank you @edoaltamura. I'll review this first to identify the changes, then I'll create a PR from my fork to the main repository.

qiskit-community / qiskit-machine-learning