Fix #885: type coercion due to slightly incorrect default value #1112
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When adding a new term to a SymbolicOperator with a sympy coefficient, the coefficient was being cast to a float. This was because the default value for a new term was 0.0, which is a float. This commit changes the default value to 0, which is an integer. This ensures that the type of the coefficient is preserved when adding new terms.
pavoljuhas
reviewed
Aug 25, 2025
Comment on lines
15
to
22
| import copy | ||
| import numpy | ||
| import sympy | ||
| import unittest | ||
| import warnings | ||
|
|
||
| import numpy | ||
| import sympy | ||
| from openfermion.config import EQ_TOLERANCE | ||
| from openfermion.testing.testing_utils import EqualsTester |
There was a problem hiding this comment.
The imports are usually grouped in 3 alphabetically sorted blocks: (1) builtin imports (2) third_party imports (3) current package imports.
Can you please put back the numpy, sympy imports after builtins?
Same in the test module above.
pavoljuhas
approved these changes
Aug 25, 2025
As pointed out by @pavoljuhas in the review comments, the ordering did not follow conventions. I ran the files through `isort` this time, to try to fix the ordering.
Yeah, I had done it by hand (the pylint setup for this project needs to be updated and doesn't help in this case) and predictably, I got it wrong. I ran the files through |
pavoljuhas
approved these changes
Aug 26, 2025
mhucka
added a commit
that referenced
this pull request
Sep 1, 2025
When adding a new term to a `SymbolicOperator` with a sympy coefficient, the coefficient was being cast to a float. This was because the default value for a new term was `0.0`, which is a float. This commit changes the default value to `0`, which is an integer. This ensures that the type of the coefficient is preserved when adding new terms.
mhucka
added a commit
that referenced
this pull request
Sep 1, 2025
When adding a new term to a `SymbolicOperator` with a sympy coefficient, the coefficient was being cast to a float. This was because the default value for a new term was `0.0`, which is a float. This commit changes the default value to `0`, which is an integer. This ensures that the type of the coefficient is preserved when adding new terms.
mhucka
added a commit
to mhucka/OpenFermion
that referenced
this pull request
Sep 25, 2025
…alue (quantumlib#1112) When adding a new term to a `SymbolicOperator` with a sympy coefficient, the coefficient was being cast to a float. This was because the default value for a new term was `0.0`, which is a float. This commit changes the default value to `0`, which is an integer. This ensures that the type of the coefficient is preserved when adding new terms.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When adding a new term to a
SymbolicOperatorwith a sympy coefficient, the coefficient was being cast to a float. This was because the default value for a new term was0.0, which is a float. This commit changes the default value to0, which is an integer. This ensures that the type of the coefficient is preserved when adding new terms.