Skip to content

history updated just if acquisition succeeded #1401

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.

Sign up for GitHub

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

Draft
lballerio wants to merge 2 commits into
base: main
Choose a base branch
from
+12 −2
Draft

history updated just if acquisition succeeded #1401

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
6e70449
changing the logic - now history is updated just if acquisition succe…
lballerio Mar 17, 2026
973d156
fixing code from review comments
lballerio Mar 17, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 12 additions & 2 deletions src/qibocal/auto/task.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,11 @@ def run(
if self.targets is None:
self.action.targets = targets

if mode is None:
# None is no iterable so code might crash later when checking
# executino modes if-statements since None is not iterable
mode = []
Comment on lines +142 to +145

@alecandido alecandido Mar 17, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

mode is a Flag enum, specifically of type ExecutionMode.

Instead of making it an empty list, which is succeeding in the current usage, but just broadening the type hint, create instead an instance of ExecutionMode.

If you want to keep it empty as a default (which is a choice), it is possible to create the equivalent enum variant, e.g. with ExecutionMode.ACQUISITION & ExecutionMode.FIT (empty intersection).
It is a bit convoluted, and because of this in py3.11 on you can do ExecutionMode(0). But we still support also py3.10

Btw: the reason not to put an empty list as default argument is because it is a mutable object, and it would be shared by all calls (search this online, you will find plenty of references and examples). However, ExecutionMode instances should not be mutable, because they are essentially integers. So, that one could be set even as default argument, and you could avoid both the Optional and the default to None. Though that's also a choice: sometimes having a special value for the unset state is useful. But, in my experience, better to avoid whenever not explicitly needed

@lballerio lballerio Apr 2, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

then what about using a tuple?

@alecandido alecandido Apr 2, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Do you mean a tuple of Booleans? That's exactly what a flag is, essentially a bitstring :)

@lballerio lballerio Apr 3, 2026

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I was more thinking about changing the default value to an empty tuple, which then is immutable and will be fine with the immutability condition, on which by the way I completely agree.
The thing is that there is a clear inconsistency in the default value for variable mode, since if you don't assign any value the code will crash because of the command:

if ExecutionMode.Something in None:

Then, I don't know if this condition might be forced or is suppressed in higher levels in the code.

Also, the main purpose of this small PR is to put acquisition as necessary and sufficient condition for updating the history in the output folder.


completed = Completed(self, folder)
try:
if platform is not None:
Expand Down Expand Up @@ -174,8 +179,13 @@ def run(
)
completed.dump_data()
if ExecutionMode.FIT in mode:
completed.results, completed.results_time = operation.fit(completed.data)
completed.dump_results()
try:
completed.results, completed.results_time = operation.fit(
completed.data
)
completed.dump_results()
except Exception as e:
log.warning(f"{e}")
return completed


Expand Down
Loading