Skip to content

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
edoaltamura opened this issue Sep 27, 2024 · 9 comments
Assignees
Labels
good first issue Good for newcomers type: design 📐 Relevant to code architecture type: enhancement ✨ Features or aspects to improve
Milestone

Comments

@edoaltamura
Copy link
Collaborator

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:
@edoaltamura edoaltamura added good first issue Good for newcomers type: design 📐 Relevant to code architecture type: enhancement ✨ Features or aspects to improve labels Sep 27, 2024
@edoaltamura edoaltamura added this to the 0.9.0 milestone Sep 27, 2024
@tsmanral
Copy link

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

@edoaltamura
Copy link
Collaborator Author

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
Copy link
Collaborator Author

Also related to linting, this old issue #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
Copy link

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.

@edoaltamura
Copy link
Collaborator Author

The syntax related to this issue might need to be extended to the V2 primitives, now merged in main, see #843.
@tsmanral please ask if there's anything you need help with! 🙂

@edoaltamura
Copy link
Collaborator Author

How is this task progressing, @tsmanral? Is there anything my team and I can help with? Could you share the link to a public branch where you are making the code changes?

@uuu4
Copy link

uuu4 commented Mar 3, 2025

Hi, can i work on this?

@edoaltamura
Copy link
Collaborator Author

Hi @uuu4, sure you can! Thanks for your interest, and looking forward to your contribution.

@uuu4
Copy link

uuu4 commented Mar 15, 2025

Thank you for assigning me! I submitted my PR and, after removing the related #pylint disable comments, the pylint score came in at 9.99/10. Being new here, the whole process was a bit nerve-wracking at first, but overall it turned out to be a really fun experience.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers type: design 📐 Relevant to code architecture type: enhancement ✨ Features or aspects to improve
Projects
None yet
Development

No branches or pull requests

3 participants