Conversation
remove: SCAN_FLAG_PV from being used, already using SCAN_FLAG fix: pvareader to use app_settings.SCAN_FLAG
pecomyint
left a comment
pecomyint
left a comment
There was a problem hiding this comment.
What's good:
- IOC_PREFIX concept is architecturally sound and the EPICS naming is correct
- Fallback hierarchy (explicit config → IOC_PREFIX + suffix → bare suffix) is clean
- POSITION simplification in hdf5_writer is reasonable (needs testing)
What needs fixing before merge:
- Wire up the consumers — pva_reader.py:665-666 and scan_view.py:428-429, 452-453 still read FILE_PATH/FILE_NAME from METADATA_CA dict. They need to switch to app_settings. FILE_PATH / app_settings.FILE_NAME, otherwise removing those keys from TOML breaks file location silently.
- Remove duplicate import — import dashpva.settings (line 17 of hdf5_writer.py) is unused.
- Fix import ordering — run ruff check --fix on the 3 changed files.
- Clean up dead motor_pos_values — lines 201-207 in hdf5_writer.py build a dict that nothing uses anymore.
- Consider renaming FILE_PATH / FILE_NAME → FILE_PATH_PV / FILE_NAME_PV to avoid ambiguity with filesystem paths.
…app_settings, to stop pulling from METADATA_CA dict remove: unused `import dashpva.settings` in hdf5_writer, to drop duplicate import fix: import ordering across 3 changed files w/ ruff, to satisfy lint remove: dead motor_pos_values dict in hdf5_writer, to drop unused code rename: FILE_PATH/FILE_NAME → FILE_PATH_PV/FILE_NAME_PV change: METADATA.CA to commented placeholders in all configs add: ROI/STATS to DEFAULT_PROFILE_DATA seed and TOMLs that lost them, to restore canonical sections add: IOC_PREFIX to aps_9id_eiger16m and aps_9id_pilatus, to align with other configs fix: HKL 'detector:' placeholders to 'ioc:' in sample_config and seed defaults so the's no confusion with prefixes remove: # NAME / # SPEC_MOTOR_NAME comments from aps_8id_eiger4m, to clean up dead lines
|
pecomyint
left a comment
pecomyint
left a comment
There was a problem hiding this comment.
Ready merge. Some minor non-blocking suggestions to do before merging
-
seed_profile_defaults_sql.py:222 — json.dumps(DEFAULT_PROFILE_DATA) uses the module-level dict directly instead of get_default_profile_data() (the deep-copy wrapper defined on the same file). Not a bug since json.dumps is read-only, but inconsistent with the file's own API.
-
Typos propagated from original TOML: INPLANE_REFERENCE_DIRECITON and SAMPLE_SURFACE_NORMAL_DIRECITON in DEFAULT_PROFILE_DATA — pre-existing typos from sample_config.toml, not introduced by this PR.
*** DIRECITON → DIRECTION (the T and I are swapped). -
config_section inconsistency: test helper _insert_profile uses '' (empty string) while seed_profile_defaults uses NULL for the same field. Doesn't affect reads since queries filter on config_type + config_key only.
fix: test_seed_defaults.py insert NULL instead of empty str
2 participants
Test Plan