Changes notification qt6#2734
Conversation
Signed-off-by: Miłosz Malczak <milosz.malczak@gmail.com>
Signed-off-by: Miłosz Malczak <milosz.malczak@gmail.com>
Signed-off-by: Miłosz Malczak <milosz.malczak@gmail.com>
Signed-off-by: Miłosz Malczak <milosz.malczak@gmail.com>
Signed-off-by: Miłosz Malczak <milosz.malczak@gmail.com>
SimonRenblad
left a comment
SimonRenblad
left a comment
There was a problem hiding this comment.
Does not account for setting arguments from applets with set_argument_value.
| modified_value_icon.setVisible_(True) | ||
|
|
||
| async def _recompute_argument(self, name): | ||
| async def _recompute_arguments(self, keys, update=True): |
There was a problem hiding this comment.
There already exists a _recompute_arguments_task in this file hooked up to the Recompute all arguments button, and as such this is confusing. Instead of modifying this function, I would call compute_expdesc directly in _update_defaults and accept the code duplication that comes with it.
| logger = logging.getLogger(__name__) | ||
|
|
||
|
|
||
| class ModifiedValueLabel(QtWidgets.QLabel): |
There was a problem hiding this comment.
Why not disable (grey out) the reset entry button if the entry is already default? That way you don't need to add another label.
|
|
||
|
|
||
| class ScanEntry(LayoutWidget): | ||
|
|
| argument = self.manager.get_submission_arguments(self.expurl)[name] | ||
| try: | ||
| procdesc = expdesc["arginfo"][name][0] | ||
| except KeyError: |
There was a problem hiding this comment.
This happens when removing an argument from the file and then refreshing the repository HEAD, correct? I am not certain that emitting a warning is the proper way to handle this. The behavior on master simply emits an error if attempting to reset to default, which also does not seem correct. Removing the widget from the EntryArea seems better to me. Whether this should be done at repository refresh or when resetting the individual entry is debatable (I lean towards the latter).
| if hash(str(self.explist.raw_view)) != hash(str(new_explist)): | ||
| logger.warning("Experiments' settings have changed") | ||
| if not compare_keys(self.explist.raw_view, new_explist): | ||
| logger.warning("Experiments' layout has changed") |
There was a problem hiding this comment.
Are these warnings really necessary?
| self.modifiedValue.emit(not self.is_default()) | ||
| self.stateChanged.connect(update) | ||
|
|
||
| def is_default(self): |
There was a problem hiding this comment.
Is it necessary to introduce a new method for this to all Entry widgets? Is it possible to compare argument["state"] with the result of default_state directly?
2 participants
ARTIQ Pull Request
Description of Changes
Add pencil sign next to the non-default experiment value.
Update the 'pencil' when there is new default value.
Add warning when settings have changed or when experiments settings layout has changed.
Related Issue
Type of Changes
Steps (Choose relevant, delete irrelevant before submitting)
All Pull Requests
git commit --signoff, see copyright).Code Changes
flake8to check code style (follow PEP-8 style).flake8has issues with parsing Migen/gateware code, ignore as necessary.Git Logistics
git rebase --interactive). Merge/squash/fixup commits that just fix or amend previous commits. Remove unintended changes & cleanup. See tutorial.git show). Format:Licensing
See copyright & licensing for more info.
ARTIQ files that do not contain a license header are copyrighted by M-Labs Limited and are licensed under LGPLv3+.