Open
Conversation
…file, and updated the main file to use the new layout.
…utton to save the processed image, and a "Reset" button to clear the current image and reset the post-processing settings. Additionally, added a dropdown menu to select different post-processing filters, and updated the layout for better user experience.
There was a problem hiding this comment.
Pull request overview
This PR updates PyBer v0.15 with a refreshed dark UI theme and a major rework of the preprocessing UI to use dockable/floating “section” panels, along with expanded trigger handling and a new “Raw signal (465)” output option.
Changes:
- Overhauled Qt stylesheet to an Adobe-like dark palette and added styling for docks/menus/tooltips.
- Refactored preprocessing UI into section popups (QDockWidgets), added workflow toolbar actions + shortcuts, and added layout persistence (QSettings + JSON import/export).
- Expanded trigger support to include analog outputs (AOUT*) alongside digital DIO channels; added “Raw signal (465)” output mode and related import/export labeling.
Reviewed changes
Copilot reviewed 6 out of 12 changed files in this pull request and generated 96 comments.
Show a summary per file
| File | Description |
|---|---|
pyBer/styles.py |
New dark palette and broader widget styling (docks, menus, tooltips). |
pyBer/main.py |
Major UI/layout persistence refactor: section docks, shortcuts, status updates, trigger map usage. |
pyBer/gui_widgets.py |
Updated label text to reflect analog + digital overlay channels. |
pyBer/gui_preprocessing.py |
New UI components (placeholder list, collapsible sections), parameter panel restructuring, overlay/threshold toggles, autorange logic. |
pyBer/analysis_core.py |
Adds analog triggers (AOUT), “Raw signal (465)” output mode, and trigger alignment improvements. |
panel_layout.json |
Added a layout JSON snapshot (currently includes geometry/state blobs). |
preprocessing_config.json |
Added a preprocessing config snapshot (appears to be an exported user config). |
pyBer/__pycache__/styles.cpython-38.pyc |
Bytecode artifact added in PR. |
pyBer/__pycache__/analysis_core.cpython-38.pyc |
Bytecode artifact added in PR. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1188
to
+1200
| self.settings.remove("post_main_dock_state_v3") | ||
| pre_state = self._to_qbytearray(self.settings.value(_PRE_DOCK_STATE_KEY, None)) | ||
| if pre_state is not None and not pre_state.isEmpty(): | ||
| if not self._is_tab_scoped_dock_state("pre", pre_state): | ||
| self.settings.remove(_PRE_DOCK_STATE_KEY) | ||
| post_state = self._to_qbytearray(self.settings.value(_POST_DOCK_STATE_KEY, None)) | ||
| if post_state is not None and not post_state.isEmpty(): | ||
| if not self._is_tab_scoped_dock_state("post", post_state): | ||
| self.settings.remove(_POST_DOCK_STATE_KEY) | ||
| except Exception: | ||
| pass | ||
|
|
||
| def _panel_config_json_path(self) -> str: |
There was a problem hiding this comment.
Persisting panel_layout.json to the repository/app install directory (computed via os.path.dirname(file)/..) is likely not writable in packaged installs and also makes layout persistence global-per-install rather than per-user. Consider storing this JSON under a user-writable location (e.g., QStandardPaths.AppConfigLocation) or keeping layout persistence entirely in QSettings, and only reading a bundled default layout as a resource/template.
Suggested change
| self.settings.remove("post_main_dock_state_v3") | |
| pre_state = self._to_qbytearray(self.settings.value(_PRE_DOCK_STATE_KEY, None)) | |
| if pre_state is not None and not pre_state.isEmpty(): | |
| if not self._is_tab_scoped_dock_state("pre", pre_state): | |
| self.settings.remove(_PRE_DOCK_STATE_KEY) | |
| post_state = self._to_qbytearray(self.settings.value(_POST_DOCK_STATE_KEY, None)) | |
| if post_state is not None and not post_state.isEmpty(): | |
| if not self._is_tab_scoped_dock_state("post", post_state): | |
| self.settings.remove(_POST_DOCK_STATE_KEY) | |
| except Exception: | |
| pass | |
| def _panel_config_json_path(self) -> str: | |
| """ | |
| Return the per-user panel layout JSON path under a writable config directory. | |
| """ | |
| # Use a per-user, writable location for persisted layout. | |
| config_dir = QtCore.QStandardPaths.writableLocation( | |
| QtCore.QStandardPaths.AppConfigLocation | |
| ) | |
| if not config_dir: | |
| # Fallback: use a directory under the user's home if QStandardPaths fails. | |
| config_dir = os.path.join(os.path.expanduser("~"), ".pyBer") | |
| try: | |
| os.makedirs(config_dir, exist_ok=True) | |
| except Exception: | |
| # If we cannot create the directory, still return the path; callers should | |
| # handle I/O failures gracefully. | |
| pass | |
| return os.path.join(config_dir, "panel_layout.json") | |
| def _load_panel_config_json_into_settings(self) -> None: | |
| """Load panel layout JSON into QSettings so existing restore logic can use it.""" | |
| # 1. Prefer a per-user layout JSON, if present. | |
| path = self._panel_config_json_path() | |
| candidate_paths = [path] | |
| # 2. Fallback to bundled default next to the application code, if any. | |
| try: | |
| base_dir = os.path.abspath(os.path.join(os.path.dirname(__file__), "..")) | |
| bundled_path = os.path.join(base_dir, "panel_layout.json") | |
| if bundled_path not in candidate_paths: | |
| candidate_paths.append(bundled_path) | |
| except Exception: | |
| pass | |
| data = None | |
| for candidate in candidate_paths: | |
| if not os.path.isfile(candidate): | |
| continue | |
| try: | |
| with open(candidate, "r", encoding="utf-8") as f: | |
| data = json.load(f) | |
| break | |
| except Exception: | |
| # Try next candidate, if any. | |
| data = None | |
| if data is None: | |
| return |
| mode = "-" | ||
| target = fs_target | ||
| try: | ||
| p = self.param_panel.get_params() |
There was a problem hiding this comment.
PlotDashboard now renders a status label (lbl_status / set_status), but MainWindow._update_plot_status() only updates the QStatusBar. This leaves the on-plot status stuck at its default text. Either call self.plots.set_status(status) here, or remove the unused label/method to avoid misleading UI.
Suggested change
| p = self.param_panel.get_params() | |
| self._show_status_message(status, 30000) | |
| # Keep the on-plot status label in sync with the status bar | |
| try: | |
| self.plots.set_status(status) | |
| except Exception: | |
| # Fail silently if plots or set_status is not available | |
| pass |
panel_layout.json
Outdated
Comment on lines
+16
to
+59
| "geometry": "AdnQywADAAD///wt///8Sv///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///8Sv///gv///4L" | ||
| }, | ||
| "filtering": { | ||
| "visible": false, | ||
| "floating": false, | ||
| "area": 2, | ||
| "geometry": "AdnQywADAAD///wt///7UP///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///7UP///gv///4L" | ||
| }, | ||
| "baseline": { | ||
| "visible": false, | ||
| "floating": false, | ||
| "area": 2, | ||
| "geometry": "AdnQywADAAAAAAWhAAADgAAAB38AAAUdAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAWhAAADgAAAB38AAAUd" | ||
| }, | ||
| "output": { | ||
| "visible": false, | ||
| "floating": false, | ||
| "area": 2, | ||
| "geometry": "AdnQywADAAD///wt///8nf///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///8nf///gv///4L" | ||
| }, | ||
| "qc": { | ||
| "visible": false, | ||
| "floating": false, | ||
| "area": 2, | ||
| "geometry": "AdnQywADAAD///wt///8nf///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///8nf///gv///4L" | ||
| }, | ||
| "export": { | ||
| "visible": false, | ||
| "floating": false, | ||
| "area": 2, | ||
| "geometry": "AdnQywADAAAAAAWhAAAFQgAAB38AAAYPAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAWhAAAFQgAAB38AAAYP" | ||
| }, | ||
| "config": { | ||
| "visible": false, | ||
| "floating": false, | ||
| "area": 8, | ||
| "geometry": "AdnQywADAAAAAAAAAAAGEgAAB38AAAZWAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAAAAAAGEgAAB38AAAZW" | ||
| } | ||
| }, | ||
| "artifact": { | ||
| "visible": false, | ||
| "floating": false, | ||
| "area": 2, | ||
| "geometry": "AdnQywADAAAAAAWhAAAAAAAAB38AAANbAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAWhAAAAAAAAB38AAANb" |
There was a problem hiding this comment.
panel_layout.json contains machine-specific geometry/state (base64 Qt saveGeometry blobs, splitter sizes, visibility) and the app also overwrites this file at runtime. If this is intended as a default layout, consider moving it into assets/resources and not writing back to it; if it's intended as user state, it should not be committed and should live in a per-user config directory.
Suggested change
| "geometry": "AdnQywADAAD///wt///8Sv///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///8Sv///gv///4L" | |
| }, | |
| "filtering": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "AdnQywADAAD///wt///7UP///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///7UP///gv///4L" | |
| }, | |
| "baseline": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "AdnQywADAAAAAAWhAAADgAAAB38AAAUdAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAWhAAADgAAAB38AAAUd" | |
| }, | |
| "output": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "AdnQywADAAD///wt///8nf///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///8nf///gv///4L" | |
| }, | |
| "qc": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "AdnQywADAAD///wt///8nf///gv///4LAAAAAAAAAAD//////////wAAAAIAAAAAB4D///wt///8nf///gv///4L" | |
| }, | |
| "export": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "AdnQywADAAAAAAWhAAAFQgAAB38AAAYPAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAWhAAAFQgAAB38AAAYP" | |
| }, | |
| "config": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 8, | |
| "geometry": "AdnQywADAAAAAAAAAAAGEgAAB38AAAZWAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAAAAAAGEgAAB38AAAZW" | |
| } | |
| }, | |
| "artifact": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "AdnQywADAAAAAAWhAAAAAAAAB38AAANbAAAAAAAAAAD//////////wAAAAIAAAAAB4AAAAWhAAAAAAAAB38AAANb" | |
| "geometry": "" | |
| }, | |
| "filtering": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "" | |
| }, | |
| "baseline": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "" | |
| }, | |
| "output": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "" | |
| }, | |
| "qc": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "" | |
| }, | |
| "export": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "" | |
| }, | |
| "config": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 8, | |
| "geometry": "" | |
| } | |
| }, | |
| "artifact": { | |
| "visible": false, | |
| "floating": false, | |
| "area": 2, | |
| "geometry": "" |
Comment on lines
+1
to
+28
| { | ||
| "artifact_detection_enabled": true, | ||
| "artifact_overlay_visible": true, | ||
| "filtering_enabled": true, | ||
| "parameters": { | ||
| "artifact_detection_enabled": true, | ||
| "artifact_mode": "Adaptive MAD (windowed)", | ||
| "mad_k": 25.0, | ||
| "adaptive_window_s": 1.0, | ||
| "artifact_pad_s": 0.5, | ||
| "lowpass_hz": 2.1, | ||
| "filter_order": 1, | ||
| "target_fs_hz": 120.0, | ||
| "baseline_method": "arpls", | ||
| "baseline_lambda": 100000000000.0, | ||
| "baseline_diff_order": 2, | ||
| "baseline_max_iter": 50, | ||
| "baseline_tol": 0.001, | ||
| "asls_p": 0.01, | ||
| "output_mode": "zscore (motion corrected with fitted ref)", | ||
| "invert_polarity": false, | ||
| "reference_fit": "OLS (recommended)", | ||
| "lasso_alpha": 0.001, | ||
| "rlm_huber_t": 1.345, | ||
| "rlm_max_iter": 50, | ||
| "rlm_tol": 1e-06 | ||
| } | ||
| } No newline at end of file |
There was a problem hiding this comment.
preprocessing_config.json looks like a user-exported configuration snapshot (parameter values) rather than source. If it's only an example, consider moving it under an examples/ or assets/ directory and documenting it; otherwise it should not be committed to avoid shipping personal/project-specific defaults unintentionally.
Suggested change
| { | |
| "artifact_detection_enabled": true, | |
| "artifact_overlay_visible": true, | |
| "filtering_enabled": true, | |
| "parameters": { | |
| "artifact_detection_enabled": true, | |
| "artifact_mode": "Adaptive MAD (windowed)", | |
| "mad_k": 25.0, | |
| "adaptive_window_s": 1.0, | |
| "artifact_pad_s": 0.5, | |
| "lowpass_hz": 2.1, | |
| "filter_order": 1, | |
| "target_fs_hz": 120.0, | |
| "baseline_method": "arpls", | |
| "baseline_lambda": 100000000000.0, | |
| "baseline_diff_order": 2, | |
| "baseline_max_iter": 50, | |
| "baseline_tol": 0.001, | |
| "asls_p": 0.01, | |
| "output_mode": "zscore (motion corrected with fitted ref)", | |
| "invert_polarity": false, | |
| "reference_fit": "OLS (recommended)", | |
| "lasso_alpha": 0.001, | |
| "rlm_huber_t": 1.345, | |
| "rlm_max_iter": 50, | |
| "rlm_tol": 1e-06 | |
| } | |
| } | |
| { | |
| "example_config": true, | |
| "description": "Example preprocessing configuration template. For production or environment-specific use, supply a separate configuration file or override these values as needed.", | |
| "artifact_detection_enabled": true, | |
| "artifact_overlay_visible": true, | |
| "filtering_enabled": true, | |
| "parameters": { | |
| "artifact_detection_enabled": true, | |
| "artifact_mode": "Adaptive MAD (windowed)", | |
| "mad_k": 25.0, | |
| "adaptive_window_s": 1.0, | |
| "artifact_pad_s": 0.5, | |
| "lowpass_hz": 2.1, | |
| "filter_order": 1, | |
| "target_fs_hz": 120.0, | |
| "baseline_method": "arpls", | |
| "baseline_lambda": 100000000000.0, | |
| "baseline_diff_order": 2, | |
| "baseline_max_iter": 50, | |
| "baseline_tol": 0.001, | |
| "asls_p": 0.01, | |
| "output_mode": "zscore (motion corrected with fitted ref)", | |
| "invert_polarity": false, | |
| "reference_fit": "OLS (recommended)", | |
| "lasso_alpha": 0.001, | |
| "rlm_huber_t": 1.345, | |
| "rlm_max_iter": 50, | |
| "rlm_tol": 1e-06 | |
| } | |
| } |
| @@ -4,6 +4,7 @@ | |||
| import os | |||
| import re | |||
| import json | |||
| import logging | |||
| from pathlib import Path | |||
| from dataclasses import dataclass | |||
There was a problem hiding this comment.
Import of 'dataclass' is not used.
Suggested change
| from dataclasses import dataclass |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| @@ -1599,15 +3636,65 @@ def _load_processed_h5(self, path: str) -> Optional[ProcessedTrial]: | |||
| ) | |||
|
|
|||
| def closeEvent(self, event): | |||
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| except Exception: | ||
| pass | ||
| try: | ||
| current = self.tabs.currentWidget() if hasattr(self, "tabs") else None |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| if current is self.pre_tab: | ||
| # Closing on preprocessing: capture the live preprocessing dock topology. | ||
| self._store_pre_main_dock_snapshot() | ||
| else: |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
| pass | ||
| try: | ||
| # Persist post layout from live state or cached tab-switch state without | ||
| # overwriting it with preprocessing topology. |
There was a problem hiding this comment.
'except' clause does nothing but pass and there is no explanatory comment.
… support of the GUI components. This should enhance the user experience and provide more stability when running the application on different platforms.
…is_core to filter the data based on the new method, and updated the main.py and gui_preprocessing.py to use the new filtering method.
…added some more files to .gitignore.
… environment.yml to include pyinstaller, and updated panel_layout.json to include the new "About" tab.
…to the post-processing options.
…hannels, and added option to export only channels with data. Also added option to export DIO channels to .xlsx. Updated GUI to reflect these changes. Updated panel layout. Updated documentation.
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
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.
No description provided.