Runmanager queue simple - #127
Open
ispielma wants to merge 93 commits into
Open
Conversation
Add a reusable ShotQueueWidget and ShotQueueTreeView in labscript_utils.qtwidgets for queue editing and state restore.
Normalize accepted_extensions consistently so a single string like '.h5' is treated as one extension instead of being iterated character by character. Make labscript_utils.qtwidgets lazily expose the new shot queue classes so importing the package no longer eagerly imports Qt-dependent code.
# Conflicts: # labscript_utils/splash.py
# Conflicts: # labscript_utils/splash.py
# Conflicts: # labscript_utils/splash.py
get_config() falls back to zprocess.zlock.DEFAULT_PORT, an int, whenever
the labconfig has no [ports] zlock entry. That value was placed directly
into the argv list passed to subprocess, which accepts only str, bytes or
os.PathLike:
TypeError: expected str, bytes or os.PathLike object, not int
So labscript-zlock could not start from a labconfig that omits the port.
The sibling launchers zlog.py and remote.py already wrap their ports with
str(); do the same here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
set_logger() stashed the currently-installed handler into
warnings._showwarning before installing logwarning. On a second
set_logger() call the stash therefore captured logwarning itself, so
logwarning recursed into itself and the next warning raised:
RecursionError: maximum recursion depth exceeded
This is reachable whenever a process installs the labscript excepthook
more than once, such as a subprocess that re-runs application startup or
an app that reconfigures logging.
Capture the real handler once at module import into a module-level name
instead, which is also idempotent and stops writing to a private-looking
attribute of the warnings module.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_default() is json.dumps' fallback for objects it cannot encode, and only
handled np.integer. np.float64 survives by accident because it subclasses
Python float, which masked the gap for the other numpy scalar types:
int64 ok
float64 ok
float32 TypeError
bool_ TypeError
Any device or plugin writing an np.float32 or np.bool_ into shot
properties therefore failed in serialise(). Add the two missing branches.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The comment directly above states the intent: only insert the
security-related kwargs when the socket is going to be a SecureSocket.
The test used SecureContext, which is a Context class and can never be a
socket class, so the branch was unreachable for any explicit
socket_class:
issubclass(zmq.Socket, SecureContext) = False SecureSocket = False
issubclass(SecureSocket, SecureContext) = False SecureSocket = True
The common pyzmq 25 path, where ThreadAuthenticator passes zmq.Socket,
gives the intended result either way, which is why this went unnoticed.
A caller that explicitly passes SecureSocket, however, silently lost its
allow_insecure configuration.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The guard that raises _ERR_NO_SHARED_SECRET was nested inside the except
clause, so it only ran when [security] allow_insecure was absent from the
labconfig. A labconfig that explicitly sets
[security]
allow_insecure = False
with no shared_secret skipped the check entirely. Inside the except the
condition was also dead weight, since allow_insecure had just been set to
False on the line above.
_ERR_NO_SHARED_SECRET states the intended contract: configure a
shared_secret, or opt out with allow_insecure = True. Explicitly writing
allow_insecure = False without a secret is precisely the misconfiguration
the message exists to catch, so run the guard on both paths.
BEHAVIOUR CHANGE: an installation that explicitly sets allow_insecure =
False with no shared_secret now fails at startup with the message above
instead of continuing. Such a setup previously worked as long as all
communication stayed on loopback, because zprocess only enforces this at
send time (zprocess/security.py). Those users must now either supply a
shared secret or set allow_insecure = True. This commit is deliberately
kept separate so it can be dropped if that trade-off is unwanted.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The TOML migration made LabConfig.get() return native types, so an option
written the natural TOML way
userlib = ['/srv/labscript/userlib', '/home/u/userlib']
comes back as a list. Two consumers still assumed the INI-era
comma-separated string and called .split(','):
AttributeError: 'list' object has no attribute 'split'
labscript_profile.add_userlib_and_pythonlib() runs from labscript-suite.pth
at *every* interpreter start, and the .pth guard only catches
ModuleNotFoundError, so the failure escaped into site.addpackage():
Error processing line 1 of .../labscript-suite.pth:
AttributeError: 'list' object has no attribute 'split'
Remainder of file ignored
Python still started, but userlib and pythonlib were never added to
sys.path, so every labscriptlib sequence and lyse analysis routine lost
its imports, with a traceback printed on every python invocation.
_get_device_dirs() has the same defect, and its result is computed at
module scope (LABSCRIPT_DEVICES_DIRS), so importing
labscript_utils.device_registry raised outright, taking BLACS, runviewer,
labscript compilation and labscript-devices with it.
Add as_config_list() next to the other shared TOML config helpers and use
it in both places. It accepts a comma-separated string, a TOML array, or a
bare scalar, so existing INI-derived configs keep working unchanged.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_resolve_appconfig_save_path() used Path.with_suffix('.toml'), which
replaces everything after the last dot rather than appending. That is only
correct when the path already ends in an app config extension, and app
config paths are routinely built from names that legitimately contain
dots.
lyse builds its plot-window settings path from the analysis script stem
and loads it back as settings_path + '.toml', so the two disagreed:
my_analysis.py save->lyse-my_analysis.toml load->lyse-my_analysis.toml same
my.analysis.v2.py save->lyse-my.analysis.toml load->lyse-my.analysis.v2.toml MISMATCH
rb.87.imaging.py save->lyse-rb.87.toml load->lyse-rb.87.imaging.toml MISMATCH
Window geometry therefore never restored for those scripts. Worse,
save_geometry() is a read-modify-write over a file it could not read, so
each plot window erased every other window's geometry, and scripts sharing
a prefix up to the last dot - rb.87.imaging.py and rb.87.absorption.py -
collapsed onto one file and overwrote each other. runviewer's
on_save_channel_config() had the same problem with user-typed filenames.
Add appconfig_path_with_suffix(), which replaces only a recognised app
config extension and appends otherwise, and route both the save and load
resolvers through it. Fixing it here means lyse and runviewer need no
change: their existing paths resolve identically to before, and the dotted
cases now agree in both directions.
_resolve_appconfig_load_path() collapses to the same three steps for every
input, which also removes a branch whose two arms returned the same value.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The INI implementation stored None as pformat(None) and read it back with
literal_eval, so None round-tripped. The TOML implementation rejects it:
save_appconfig({'s': {'k': None}})
-> TypeError: s/k value None is not representable in TOML app config
Nothing in any app's save_configuration() catches that, so a single None
anywhere in the saved state turns "Save configuration" into an unhandled
traceback. No current payload produces one, so this is a latent regression
rather than a live failure, but it is a fuse on the save path for any new
or plugin-contributed state.
TOML has no null and expresses absence by omitting the key, so omit
options whose value is None. load_appconfig() returns a mapping and every
consumer reads optional state with .get(), which yields None again, so the
round trip is preserved:
{'k': None, 'kept': 1} -> file has only `kept` -> .get('k') is None
None nested inside a list still raises, because dropping an element there
would shift every later index. The message now says so explicitly instead
of reporting it as an unrepresentable value.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
save_appconfig() writes whatever sections it is given, but
load_appconfig() dropped any table named 'default', so the round trip lost
data with no error:
saved {'default': {'k': 1}, 'other': {'j': 2}}
loaded {'other': {'j': 2}}
App configs are plain section/option documents. Unlike a labconfig they
have no DEFAULT inheritance, so the name carries no special meaning and
there is nothing to filter out. The INI implementation returned it too, so
this also restores the earlier behaviour.
No application currently writes a section by that name, so this is latent,
but the asymmetry is a trap for anything that later does.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PluginRefactor lifted this loop out of BLACS and reproduced the defect it
carried. Config sections inherit the defaults section, so
config.items(section) returns the section's own options plus every
labconfig default. Against a real labconfig ten non-plugin keys are seen
as configured plugins:
analysislib, app_saved_configs, apparatus_name, experiment_shot_storage,
labscript_suite, labscriptlib, pythonlib, shared_drive, user_devices, userlib
A plugin directory whose name collides with one of those never had an
enable flag written into the section, so getboolean() read the default's
value instead:
ValueError: Not a boolean: /some/path/userlib
That call was unguarded, and blacs/plugins/__init__.py calls
discover_modules() at module scope, so `import blacs.plugins` raised and
BLACS did not start at all rather than skipping one plugin.
Ask has_option() instead of scanning items(), and write a real flag into
the section when the inherited value is not a boolean; a section option
shadows the defaults, so the plugin resolves from then on. An installation
that sets the flag in both places is unaffected, since the section value
already wins:
collision only in defaults before: ValueError after: starts, plugin disabled
set in both places before: enabled after: enabled
This is the labscript-utils counterpart of blacs 87c3774, which fixed the
same code before PluginRefactor moved it here.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Newly discovered plugins were seeded with str(bool), which was harmless
under INI where every value is a string, but writes a quoted string into a
TOML config beside hand-written booleans:
connection_table = true # written by hand
cycle_time = "False" # written by discover_modules
getboolean() still reads it correctly, so nothing broke, but the config
file ends up internally inconsistent and a user editing it sees two
spellings of the same flag. TomlConfigParser preserves native types, so
pass the bool through:
connection_table = true
cycle_time = false
theme = true
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
PluginRefactor carried BLACS's old-API fallback across unchanged: the
retry is driven by catching Exception from the call itself, so any failure
inside a correctly-written plugin_setup_complete(data) triggers a second
invocation.
That matters more here than it did in BLACS, because BasePlugin in this
module declares exactly the signature that double-runs:
def plugin_setup_complete(self, data=None)
For that shape the retry re-enters the body, so whatever the first,
partially completed call had already done - started a thread, registered a
listener, opened a connection - happens again:
plugin_setup_complete(self, data=None) before: side effect ran 2x
after: side effect ran 1x
Plugins with a strict arity were never double-run, because the retry
failed on argument count before reaching the body.
Bind the signature first and call once with the arguments it accepts. Old
no-argument plugins still work and still get the deprecation warning;
plugins with no introspectable signature fall back to the current API. A
genuine error now produces one clear log entry instead of a misleading
"Trying again with old call signature" followed by an arity error.
This is the labscript-utils counterpart of blacs e3e5d2f.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two problems in MenuContext.render().
Group order was assigned first-seen:
if group not in group_orders[key]:
group_orders[key][group] = len(group_orders[key])
First-seen order follows self.contributions, which follows self.plugins,
which follows os.listdir(self.plugins_dir). Every other component of the
sort key is deterministic, so this one made menu group order depend on the
filesystem:
discovery order 1 -> ['M-act', '---', 'A-act', '---', 'Z-act']
discovery order 2 -> ['Z-act', '---', 'A-act', '---', 'M-act'] (before)
-> ['M-act', '---', 'A-act', '---', 'Z-act'] (after)
Sort group names instead, with an ungrouped contribution first.
render() also never cleared self.contributions, so calling it a second
time duplicated every entry. Consume them, which leaves a repeat call a
no-op.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
_get_contributions() accepts any iterable, including a generator, but
setup_contexts() then tests it for emptiness:
if menu_contributions and 'menus' not in self.contexts:
self.logger.error("... provided menu contributions, but no 'menus'
context is registered. Skipping.")
continue
A generator is truthy even when it yields nothing, so a plugin that
contributed no menus was reported as having contributed some, and the
continue skipped the rest of that plugin's routing.
Return a list, which makes emptiness meaningful and lets the result be
iterated more than once. A generator that raises while being consumed is
logged like any other malformed contribution rather than escaping into the
caller.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two problems, one of which explains the other. tests/test_plugins.py uses the pytest caplog fixture, but pytest was not declared anywhere - there was only a docs extra - so there was no supported way to run these tests at all. More importantly, FakeConfig diverged from a real LabConfig in exactly the two ways that matter for plugin discovery. It returned only a section's own options, modelling none of the defaults inheritance that caused the defaults-as-plugin-names bug, and its getboolean() called .lower() on the value, assuming the INI-era strings rather than the native booleans TOML returns. A fake that cannot express the bug cannot catch it. Add a test extra with pytest, teach FakeConfig both behaviours, and add regression tests for the fixes in this series: - a config default is never read as a plugin's enable flag - an explicit flag set in both places still wins - seeded flags are written as booleans - an optional-argument plugin_setup_complete() runs exactly once - empty generator contributions do not report a missing context - menu group order is independent of discovery order - render() is idempotent The pre-existing discovery test asserted the old str(bool) seeding and is updated to expect real booleans, matching the change in d630999. 22 passed Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two files needed hand resolution; both sides were kept.
labscript_utils/plugins.py: this branch replaced setup_complete() with the
ordered setup-activity pipeline and added collect_services(), so
Production's rewrite of the older loop could not be taken as-is. The
pipeline is kept and the signature-detection fix is applied inside its
runner instead, as _setup_action_args(). The legacy no-argument fallback
is still offered for plugin_setup_complete activities only; any other
activity whose action does not accept data keeps raising, so a genuine
plugin bug still surfaces rather than being silently called differently.
BasePlugin-shaped hook side effect ran 1x (was 2x)
legacy no-arg hook still runs, still warns
multi-activity pipeline priority order preserved
tests/test_plugins.py: both branches appended independent tests. Kept
both.
31 passed
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
runmanager formats these templates in two passes: once in
new_sequence_details() with preserve_unresolved_roots=('globals',), and
again per shot to resolve globals. The first pass un-escaped braces, as
any formatter does, so its output was not a valid template for the second:
'run{{special}}/{sequence_index:04d}' pass1='run{special}/0007'
pass2-> UnresolvedLookupError: special
A resolved value containing a brace failed the same way, since the second
pass read it as a field:
script_basename='my{script}' pass1='my{script}/0007'
pass2-> UnresolvedLookupError: script
Preserved placeholders only mean anything to a later pass, so enabling
preservation is exactly the signal that this pass is producing a template.
In that case escape literal text and resolved values, leaving preserved
placeholders parseable; with no preservation the result is final text and
is emitted verbatim.
'run{{special}}/{sequence_index:04d}' pass1='run{{special}}/0007'
pass2='run{special}/0007'
script_basename='my{script}' pass2='my{script}/0007'
Output for templates without literal braces is unchanged, which covers
every shipped example.
One consequence worth knowing: runmanager shows the first-pass result in
its shot output folder field, so a template written with literal {{ }}
now displays them escaped there. That only affects templates that contain
literal braces, which previously could not be used at all.
_UnresolvedLookupError is renamed UnresolvedLookupField and is now public,
so callers can catch an unresolved lookup by name.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
format_lookup_string() escapes braces when preservation is enabled, since
the result is a template for a further pass that will unescape them again.
A caller that displays the intermediate rather than formatting it needs to
do that unescaping itself:
'run{{special}}/{sequence_index:04d}' -> template 'run{{special}}/0007'
display 'run{special}/0007'
Expose unescape_braces() beside _escape_braces() so the pair stays in sync,
rather than having callers hand-roll the inverse. Preserved placeholders
are untouched, so a preview still shows unresolved globals[...] lookups as
written.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each unhandled exception spawns a tkinter subprocess window, up to MAX_WINDOWS at a time. All four GUI apps and lyse's analysis subprocess import this module in their first few lines, so a debugging run can leave many windows to close by hand. Guard the window-spawning block on LABSCRIPT_NO_ERROR_DIALOG (or the NO_ERROR_DIALOG module attribute, for toggling at runtime). The guard sits inside tkhandler rather than around the sys.excepthook installation, so exceptions are still logged and still reach stderr when the dialog is off. The default is unchanged: standard labscript runs keep the dialog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Each unhandled exception spawns a tkinter subprocess window, up to MAX_WINDOWS at a time. All four GUI apps and lyse's analysis subprocess import this module in their first few lines, so a debugging run can leave many windows to close by hand. Guard the window-spawning block on LABSCRIPT_NO_ERROR_DIALOG (or the NO_ERROR_DIALOG module attribute, for toggling at runtime). The guard sits inside tkhandler rather than around the sys.excepthook installation, so exceptions are still logged and still reach stderr when the dialog is off. The default is unchanged: standard labscript runs keep the dialog. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Point the docs extra at PyQt6 and raise two dependency floors that were too low to actually support it: qtutils to 4.1.3, the release that added the PyQt6 shim and what the rest of the suite already requires, and pyqtgraph to 0.13.0. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Keeps the feature branch current with Production. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The suite targets PyQt6 only, so remove the version gates rather than
carrying two code paths:
- splash.py: the PyQt5-only AA_EnableHighDpiScaling / AA_UseHighDpiPixmaps
block; both are default in Qt6 and warn if set.
- enumoutput.py, digitaloutput.py: the 'QT_ENV != PYQT5' gate on the
scroll-wheel passthrough.
- dragdroptab.py: debug prints for PyQt4/PyQt5.
The wheel passthrough those gates guarded had been dead since Qt4 -- it
called pos(), globalPos(), delta() and orientation() on a QWheelEvent, and
delta()/orientation() were removed back in Qt5, so the branch raised
AttributeError on every binding. Rebuilt on the Qt6 API (position(),
globalPosition(), pixelDelta(), angleDelta(), phase(), inverted()); the
QPointF conversion is required because mapToParent() returns a QPoint.
Also switch the shot-queue context menu from QMenu.exec_() to exec():
qtutils only aliases exec_ onto QCoreApplication and QDialog, so QMenu
raised AttributeError on right-click under PyQt6.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
exec_() exists under PyQt6 only because qtutils aliases it onto QCoreApplication and QDialog. The suite targets PyQt6 only, so call the real method and stop depending on that shim. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
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.
Added ability for control of startup order by priority.