Skip to content

Conversation

Copy link

Copilot AI commented Dec 15, 2025

The ImportCSVPopup class contained significant code duplication across field initialization, UI creation, and event handlers.

Changes

  • Consolidated field storage: Replaced 11 individual self.selected_* attributes with a single self.selected_fields dictionary
  • Extracted UI builder: Created _create_field_layout() helper to generate field layouts from configuration tuples, replacing ~140 lines of duplicated widget creation code with a 12-item config list
  • Unified event handlers: Replaced 11 nearly identical select_*() methods with a single _generic_select_handler() that dispatches based on field name
  • Simplified data collection: collate_selections() now returns self.selected_fields.copy() instead of manually reconstructing the dictionary

Example

Before:

# Repetitive field creation
filepath_layout = QHBoxLayout()
filepath_layout.addWidget(QLabel("Filepath:"))
asterisk = QLabel("*")
asterisk.setStyleSheet("QLabel { color : red; }")
filepath_layout.addWidget(asterisk, alignment=Qt.AlignmentFlag.AlignRight)
self.filepath = QComboBox()
self.filepath.addItems(self.columns)
self.filepath.currentTextChanged.connect(self.select_filepath)
# ... repeated 10 more times

After:

field_configs = [
    ("Filepath:", "filepath", True, False),
    ("Timestamp:", "timestamp", True, False),
    # ... 9 more fields
]

for label_text, field_name, is_required, use_separator in field_configs:
    field_layout = self._create_field_layout(label_text, field_name, items, is_required, use_separator)
    layout.addLayout(field_layout)

Net reduction: 314 → 235 lines (-25%)

Legacy select_*() methods preserved as thin wrappers for backward compatibility.

Original prompt

Refactor the ImportCSVPopup class to improve its efficiency by reducing repetitive code and making it cleaner and more maintainable.

This pull request was created as a result of the following prompt from Copilot chat.

Refactor the ImportCSVPopup class to improve its efficiency by reducing repetitive code and making it cleaner and more maintainable.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

Copilot AI and others added 2 commits December 15, 2025 21:55
Co-authored-by: tkswanson <4371698+tkswanson@users.noreply.github.com>
Co-authored-by: tkswanson <4371698+tkswanson@users.noreply.github.com>
Copilot AI changed the title [WIP] Refactor ImportCSVPopup class for improved efficiency Refactor ImportCSVPopup to eliminate repetitive code patterns Dec 15, 2025
Copilot AI requested a review from tkswanson December 15, 2025 21:59
@tkswanson tkswanson changed the base branch from main to animl-lite December 16, 2025 18:37
@tkswanson
Copy link
Collaborator

@copilot add the changes to popup_import_csv.py to the branch popup-import-update

Copy link
Author

Copilot AI commented Dec 16, 2025

@copilot add the changes to popup_import_csv.py to the branch popup-import-update

I'm unable to directly create or switch to the popup-import-update branch and apply these changes there, as my tooling doesn't support branch creation or switching.

The refactoring changes are currently in the copilot/refactor-import-csv-popup branch. Could you clarify how you'd like me to proceed? Options:

  1. You could create the popup-import-update branch from copilot/refactor-import-csv-popup, or
  2. If you want these changes merged into an existing popup-import-update branch, you could create that branch and let me know

Base automatically changed from animl-lite to dev January 16, 2026 21:45
@tkswanson tkswanson closed this Jan 16, 2026
@tkswanson tkswanson deleted the copilot/refactor-import-csv-popup branch January 16, 2026 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants