Skip to content

Conversation

mhucka
Copy link
Collaborator

@mhucka mhucka commented Sep 6, 2025

Some of the code used type(X) == Y to compare types. Those constructs lead to warnings from flake8 because type comparisons should use isinstance.

Some of the code used `type(X) == Y` to compare types. Those constructs
lead to warnings from `flake8`. It is better to use `isinstance`.
cirq.Moment(cirq.H.on_each(pair[1] for pair in pairs_list)),
]
elif type(depolarization_probability) == float:
elif isinstance(depolarization_probability, float):
Copy link
Collaborator

@pavoljuhas pavoljuhas Sep 15, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(optional) consider changing to numbers.Real if we'd like to handle np.float32 values.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good idea. Since there may be other places in the code where a similar change ought to be made, I won't do it in this PR, and instead I opened an issue (#435) to remind us to do a broader check for numerical type comparisons that might not behave as expected with NumPy 2.

Copy link
Collaborator

@pavoljuhas pavoljuhas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please address comments in recirq/fermi_hubbard/parameters.py before merging.

Per [comments by @pavoljuhas in
review](quantumlib#430 (comment)),
this rewrites a couple of lines to simplify a comparisons.
@mhucka mhucka enabled auto-merge (squash) September 18, 2025 23:01
@mhucka mhucka merged commit a9b1c7e into quantumlib:master Sep 18, 2025
11 checks passed
@mhucka mhucka changed the title Fix type comparisons to use isinstance Fix #435: change type comparisons to use isinstance Sep 19, 2025
@mhucka mhucka changed the title Fix #435: change type comparisons to use isinstance Change type comparisons to use isinstance Sep 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants