-
Notifications
You must be signed in to change notification settings - Fork 335
[wip] Refactor do2d #7066
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
base: main
Are you sure you want to change the base?
[wip] Refactor do2d #7066
Conversation
num_points2: int, | ||
delay2: float, | ||
*param_meas: ParamMeasT, | ||
set_before_sweep: bool | None = True, |
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.
flush_columns and set_before_sweep are not supported by dond yet. Need to figure out how to deal with that
2d5634b
to
9b3588f
Compare
message is used. | ||
break_condition: Callable that takes no arguments. If returned True, | ||
measurement is interrupted. | ||
**kwargs: kwargs are the same as for dond and forwarded directly to dond. |
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.
Add links like do0d/do1d
if log_info is not None: | ||
meas._extra_log_info = log_info | ||
else: | ||
meas._extra_log_info = "Using 'qcodes.dataset.do2d'" |
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.
Retain this log message like do1d/do0d
enter_actions: ActionsT = (), | ||
exit_actions: ActionsT = (), | ||
before_inner_actions: ActionsT = (), | ||
after_inner_actions: ActionsT = (), |
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.
These are also not available.
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.
enter_actions
and exit_actions
are available in dond
.
Here's a suggestion to include before_inner_actions
and after_inner_actions
:
- Add arguments
pre_sweep_actions
andpost_sweep_actions
toLinSweep
- In
_Sweeper.__getitem__
, check if index is first or last in the sweep, then appendpre_sweep
orpost_sweep
to the actions accordingly.
Happy to have a go at this in a separate PR if you think it's a good solution
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.
Pull Request Overview
This PR refactors the do2d function to delegate its core functionality to the dond function by using LinSweep objects.
- Replaces legacy measurement setup, plotting, and parameter registration code
- Delegates parameter sweep management and configuration through keyword arguments via dond
Comments suppressed due to low confidence (1)
src/qcodes/dataset/dond/do_2d.py:65
- The refactoring removes several measurement configuration steps (e.g., parameter registrations, write period handling, and logging extra info) that were present in the legacy implementation; ensure that these functionalities are fully handled by dond or reintegrate the missing behavior if needed.
return cast(
show_progress: bool | None = None, | ||
log_info: str | None = None, | ||
break_condition: BreakConditionT | None = None, | ||
**kwargs: Unpack[DondKWargs], |
Copilot
AI
May 19, 2025
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 function docstring has not been updated to reflect the new keyword arguments that are forwarded to dond; consider updating it to clearly document the accepted parameters.
Copilot uses AI. Check for mistakes.
Resolves #7076