-
Notifications
You must be signed in to change notification settings - Fork 19
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
Fix uninitialised attributes in QCVV samples #1075
Conversation
} | ||
) | ||
else: | ||
warnings.warn("Sample is missing probabilities. This sample has been omitted.") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we might not want to repeat this warning for every missing sample
|
||
for sample in tqdm.notebook.tqdm(samples, desc="Evaluating circuits"): | ||
sample.target_probabilities = self._simulate_sample(sample) | ||
|
||
records = [] | ||
for sample in samples: | ||
target_probabilities = { | ||
f"p({key})": value for key, value in sample.target_probabilities.items() | ||
f"p({key})": value | ||
for key, value in sample.target_probabilities.items() # type: ignore[union-attr] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is it possible to avoid the type: ignore pragmas? e.g. with
for key, value in sample.target_probabilities.items() # type: ignore[union-attr] | |
for key, value in sample.target_probabilities.items() if sample.target_probabilities is not None |
or by combining this w/ the previous loop
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The logic meant that this case was never actually reached, hence the type: ignore. Just adding the not None
check to the list comprehension doesn't work unfortunately so I've added an explicit check in the loop instead.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm!
Closes #1071