Skip to content

Conversation

t-b
Copy link
Collaborator

@t-b t-b commented Jul 16, 2025

Close #2464

@t-b t-b self-assigned this Jul 16, 2025
@timjarsky timjarsky requested a review from Copilot August 5, 2025 19:09
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds support for using ipfx routines from within MIES and applies code formatting changes across multiple files. The main purpose is to integrate ipfx spike feature extraction capabilities into the MIES Igor Pro environment.

Key changes include:

  • Added Python integration for ipfx spike extraction functionality
  • Applied consistent code formatting using Black formatter
  • Removed several GitHub workflow files

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
tools/nwb-read-tests/nwbv2-read-test.py Applied Black code formatting to improve consistency
Packages/doc/conf.py Applied Black formatting and improved code structure
Packages/Python/ipfx_helpers.py New Python module for ipfx spike feature extraction
Packages/MIES/MIES_Python.ipf New Igor Pro function to call Python spike extractor
Packages/MIES_Include.ipf Added include for new Python module
.pre-commit-config.yaml Added Black formatter configuration
.github/workflows/*.yml Removed multiple workflow files
Comments suppressed due to low confidence (1)

.pre-commit-config.yaml:39

  • The Black version 25.1.0 does not exist. As of January 2025, the latest Black version is in the 24.x series. Please use a valid version like 24.10.0.
  rev: 25.1.0

Comment on lines +10 to +11
def extract_spikes(t_path: str, v_path: str, out_folder: str):

Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function lacks a docstring explaining its purpose, parameters, and return value. Consider adding documentation that describes what this function does, the expected format of the input paths, and what the return value represents.

Suggested change
def extract_spikes(t_path: str, v_path: str, out_folder: str):
def extract_spikes(t_path: str, v_path: str, out_folder: str):
"""
Extract spike features from voltage and time data files and save the results as a text wave.
Parameters:
t_path (str): Path to the time data file (Igor Pro wave file).
v_path (str): Path to the voltage data file (Igor Pro wave file).
out_folder (str): Path to the output folder where the result will be saved.
Returns:
str: The name of the result wave containing spike features, or an empty string if no spikes are found.
"""

Copilot uses AI. Check for mistakes.


# empty list are not handled correctly, reported as #7234
if spikes.empty:
return ""
Copy link

Copilot AI Aug 5, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[nitpick] The magic return value of empty string for no spikes could be confusing. Consider using a more explicit approach like returning None or raising an exception with a descriptive message.

Suggested change
return ""
return None

Copilot uses AI. Check for mistakes.

@timjarsky timjarsky requested a review from Copilot August 7, 2025 17:44
Copy link

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

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.

Playing around with python support in IP10 with ipfx

1 participant