qiboteam / qibo

A full-stack framework for quantum computing.
https://qibo.science
Apache License 2.0
294 stars 60 forks source link

Tuple-related type hints #1420

Closed alecandido closed 3 months ago

alecandido commented 3 months ago

A quite trivial fix, just because I noticed it while working on Qibolab.

Tuples are immutable containers, thus the plain type hint is dedicated to the fixed size ones. To return a variable size tuple, tuple[MyType, ...] is required. tuple[MyType] is describing a 1-element tuple, whose element has type MyType.

Conversely, lists and dictionaries are mutable, and their basic and only type hints are not imposing any restriction on the length.

codecov[bot] commented 3 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 99.94%. Comparing base (4f57f37) to head (e1179f8). Report is 3 commits behind head on master.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #1420 +/- ## ======================================= Coverage 99.94% 99.94% ======================================= Files 78 78 Lines 11297 11297 ======================================= Hits 11291 11291 Misses 6 6 ``` | [Flag](https://app.codecov.io/gh/qiboteam/qibo/pull/1420/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | Coverage Δ | | |---|---|---| | [unittests](https://app.codecov.io/gh/qiboteam/qibo/pull/1420/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam) | `99.94% <100.00%> (ø)` | | Flags with carried forward coverage won't be shown. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=qiboteam#carryforward-flags-in-the-pull-request-comment) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

renatomello commented 3 months ago

These changes would have to be applied throughout the entire module, no?

alecandido commented 3 months ago

These changes would have to be applied throughout the entire module, no?

Yes, and I'm planning to do a consistent review of types at some point (there are further improvements/fixes to add). But these methods are very common, so I decided to fix these immediately, instead of delaying them with everything else.

Moreover, I checked that most of the usage of Tuple[] are consistent, since they refer to fixed length tuples. There might be some further occurrences just in the quantum_info module.

alecandido commented 3 months ago

@renatomello: for me, this PR is ready as it is. But if you want to do something else, we can discuss (though I will not commit soon to much more, that's why I'd prefer to open a new one later on).

BrunoLiegiBastonLiegi commented 3 months ago

I wasn't aware of the Tuple[type, ...] syntax, thanks for pointing out.