Deprecate positional conversion options in `add_to_nwbfile` and interface init by h-mayorquin · Pull Request #1663 · catalystneuro/neuroconv

h-mayorquin · 2026-02-13T17:05:42Z

Allows only nwbfile and metadata for add_to_nwbfile
Allows file_path, file_paths and folder_path for __init__

Copilot

Pull request overview

This pull request enforces keyword-only arguments for conversion options across all data interfaces in NeuroConv. The changes ensure that __init__ methods only accept file_path, folder_path, or file_paths as positional arguments, while add_to_nwbfile methods only accept nwbfile and metadata positionally. All other parameters must be passed as keyword arguments.

Changes:

Added deprecation handling using *args pattern in __init__ and add_to_nwbfile methods across ~100 data interfaces
Replaced existing * keyword-only markers with *args temporarily to maintain backward compatibility during deprecation period
Updated super() calls to use keyword arguments where necessary to avoid passing positional arguments to parent methods

Reviewed changes

Copilot reviewed 70 out of 70 changed files in this pull request and generated 10 comments.

File	Description
src/neuroconv/datainterfaces/*/.py	Added `args` deprecation pattern with FutureWarning for positional conversion options in init* and add_to_nwbfile methods across all interface files
docs/developer_guide/style_guide.rst	Documented the new convention requiring keyword-only arguments for conversion options with code examples
CHANGELOG.md	Added entry documenting the keyword-only argument enforcement and deprecation timeline

Comments suppressed due to low confidence (2)

src/neuroconv/datainterfaces/ophys/femtonics/femtonicsdatainterface.py:161

This import of module warnings is redundant, as it was previously imported on line 3.
src/neuroconv/datainterfaces/ecephys/cellexplorer/cellexplorerdatainterface.py:246
This class does not call BaseRecordingExtractorInterface.init during initialization. (CellExplorerRecordingInterface.init may be missing a call to a base class init)

class CellExplorerRecordingInterface(BaseRecordingExtractorInterface):

Copilot · 2026-02-13T17:52:34Z

CHANGELOG.md

 # v0.9.2 (Upcoming)

 ## Removals, Deprecations and Changes
+* Enforced keyword-only arguments across all data interfaces: `__init__` methods now only accept `file_path`/`folder_path`/`file_paths` as positional arguments, and `add_to_nwbfile` methods only accept `nwbfile` and `metadata` as positional arguments. Existing positional usage in `add_to_nwbfile` will emit a `FutureWarning` and will be removed on or after August 2026. [PR #1663](https://github.com/catalystneuro/neuroconv/pull/1663)


The CHANGELOG entry should mention that both __init__ and add_to_nwbfile methods will emit FutureWarning for positional argument usage. Currently, it only mentions add_to_nwbfile. Consider updating to: "Existing positional usage in both __init__ and add_to_nwbfile will emit a FutureWarning and will be removed on or after August 2026."

Suggested change

* Enforced keyword-only arguments across all data interfaces: `__init__` methods now only accept `file_path`/`folder_path`/`file_paths` as positional arguments, and `add_to_nwbfile` methods only accept `nwbfile` and `metadata` as positional arguments. Existing positional usage in `add_to_nwbfile` will emit a `FutureWarning` and will be removed on or after August 2026. [PR #1663](https://github.com/catalystneuro/neuroconv/pull/1663)

* Enforced keyword-only arguments across all data interfaces: `__init__` methods now only accept `file_path`/`folder_path`/`file_paths` as positional arguments, and `add_to_nwbfile` methods only accept `nwbfile` and `metadata` as positional arguments. Existing positional usage in both `__init__` and `add_to_nwbfile` will emit a `FutureWarning` and will be removed on or after August 2026. [PR #1663](https://github.com/catalystneuro/neuroconv/pull/1663)

Copilot · 2026-02-13T17:52:35Z

src/neuroconv/datainterfaces/icephys/abf/abfdatainterface.py

@@ -1,4 +1,5 @@
 import json
+import warnings


Module 'warnings' is imported with both 'import' and 'import from'.

Copilot · 2026-02-13T17:52:35Z

src/neuroconv/datainterfaces/ecephys/axon/axondatainterface.py

@@ -1,3 +1,4 @@
+import warnings


Module 'warnings' is imported with both 'import' and 'import from'.

Suggested change

import warnings

Copilot · 2026-02-13T17:52:35Z

src/neuroconv/datainterfaces/ecephys/openephys/openephyslegacydatainterface.py

@@ -1,3 +1,4 @@
+import warnings


Module 'warnings' is imported with both 'import' and 'import from'.

Copilot · 2026-02-13T17:52:36Z

src/neuroconv/datainterfaces/icephys/baseicephysinterface.py

+                FutureWarning,
+                stacklevel=2,
+            )
+            icephys_experiment_type = positional_values.get("icephys_experiment_type", icephys_experiment_type)


Variable icephys_experiment_type is not used.

Copilot · 2026-02-13T17:52:36Z

src/neuroconv/datainterfaces/ecephys/neuroscope/neuroscopedatainterface.py

+            )
+            gain = positional_values.get("gain", gain)
+            xml_file_path = positional_values.get("xml_file_path", xml_file_path)
+            verbose = positional_values.get("verbose", verbose)


Variable verbose is not used.

Copilot · 2026-02-13T17:52:36Z

src/neuroconv/datainterfaces/ecephys/neuroscope/neuroscopedatainterface.py

+            gain = positional_values.get("gain", gain)
+            xml_file_path = positional_values.get("xml_file_path", xml_file_path)
+            verbose = positional_values.get("verbose", verbose)
+            es_key = positional_values.get("es_key", es_key)


Variable es_key is not used.

Copilot · 2026-02-13T17:52:36Z

src/neuroconv/datainterfaces/ecephys/openephys/openephysbinarydatainterface.py

+            )
+            stream_name = positional_values.get("stream_name", stream_name)
+            block_index = positional_values.get("block_index", block_index)
+            stub_test = positional_values.get("stub_test", stub_test)


Variable stub_test is not used.

I think it is. can you double check this? @copilot

Copilot · 2026-02-13T17:52:36Z

src/neuroconv/datainterfaces/ophys/suite2p/suite2pdatainterface.py

+            include_roi_centroids = positional_values.get("include_roi_centroids", include_roi_centroids)
+            include_roi_acceptance = positional_values.get("include_roi_acceptance", include_roi_acceptance)
+            mask_type = positional_values.get("mask_type", mask_type)
+            plane_segmentation_name = positional_values.get("plane_segmentation_name", plane_segmentation_name)


Variable plane_segmentation_name is not used.

Copilot · 2026-02-13T17:52:37Z

src/neuroconv/datainterfaces/ecephys/axona/axonadatainterface.py

+            )
+            noise_std = positional_values.get("noise_std", noise_std)
+
        super().__init__(filename=file_path, noise_std=noise_std)


Keyword argument 'filename' is not a supported parameter name of method AxonaRecordingInterface.init.
Keyword argument 'noise_std' is not a supported parameter name of method AxonaRecordingInterface.init.

Suggested change

super().__init__(filename=file_path, noise_std=noise_std)

super().__init__(file_path=file_path)

Copilot · 2026-02-13T18:01:08Z

@h-mayorquin I've opened a new pull request, #1665, to work on those changes. Once the pull request is ready, I'll request review from you.

pauladkisson

It was a lot of files, so I wanted to try out Copilot review to see if it would catch anything that I missed. As far as I can tell, the changes look good. Copilot's review seems to have caught a bunch of off-target stuff. Some of which may be legitimate and others that are really not. So I'll let you decide what to do with all that.

h-mayorquin · 2026-02-13T18:37:55Z

It was a lot of files, so I wanted to try out Copilot review to see if it would catch anything that I missed. As far as I can tell, the changes look good. Copilot's review seems to have caught a bunch of off-target stuff. Some of which may be legitimate and others that are really not. So I'll let you decide what to do with all that.

Thanks, this makes sense. That's partly why I shared with you #1664 as an entry point (which included these changes.

Yes, I checked some of the github copilot review and I think it went off the rails.

Thanks for the review.

codecov · 2026-02-13T19:13:54Z

Codecov Report

❌ Patch coverage is 14.68468% with 947 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.18%. Comparing base (9f2c0e6) to head (80ba461).
⚠️ Report is 2 commits behind head on main.

Files with missing lines	Patch %	Lines
.../ecephys/cellexplorer/cellexplorerdatainterface.py	10.00%	45 Missing ⚠️
...tainterfaces/ecephys/plexon/plexondatainterface.py	15.90%	37 Missing ⚠️
...aces/ecephys/neuroscope/neuroscopedatainterface.py	10.81%	33 Missing ⚠️
...s/ophys/miniscope/miniscopeimagingdatainterface.py	12.50%	28 Missing ⚠️
...datainterfaces/ophys/minian/miniandatainterface.py	10.71%	25 Missing ⚠️
...tainterfaces/ophys/suite2p/suite2pdatainterface.py	10.71%	25 Missing ⚠️
...terfaces/ecephys/spikeglx/spikeglxnidqinterface.py	7.69%	24 Missing ⚠️
...aces/ophys/scanimage/scanimageimaginginterfaces.py	7.69%	24 Missing ⚠️
...cephys/openephys/openephybinarysanaloginterface.py	8.00%	23 Missing ⚠️
...v/datainterfaces/ecephys/edf/edfanaloginterface.py	8.33%	22 Missing ⚠️
... and 53 more

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1663      +/-   ##
==========================================
- Coverage   86.52%   80.18%   -6.34%     
==========================================
  Files         159      159              
  Lines       11539    12624    +1085     
==========================================
+ Hits         9984    10123     +139     
- Misses       1555     2501     +946

Flag	Coverage Δ
unittests	`80.18% <14.68%> (-6.34%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...s/behavior/lightningpose/lightningposeconverter.py	`80.24% <ø> (ø)`
...ainterfaces/ecephys/maxwell/maxonedatainterface.py	`100.00% <100.00%> (ø)`
...onv/datainterfaces/ecephys/tdt/tdtdatainterface.py	`69.76% <ø> (ø)`
...ainterfaces/ophys/baseimagingextractorinterface.py	`77.01% <ø> (ø)`
...rfaces/ophys/basesegmentationextractorinterface.py	`79.31% <ø> (ø)`
...faces/behavior/miniscope/miniscopedatainterface.py	`84.61% <27.27%> (-15.39%)`	⬇️
...datainterfaces/ophys/caiman/caimandatainterface.py	`71.42% <27.27%> (-28.58%)`	⬇️
...v/datainterfaces/ophys/cnmfe/cnmfedatainterface.py	`65.21% <27.27%> (-34.79%)`	⬇️
...phys/inscopix/inscopixsegmentationdatainterface.py	`83.82% <20.00%> (-5.07%)`	⬇️
.../micromanagertiff/micromanagertiffdatainterface.py	`80.95% <27.27%> (-19.05%)`	⬇️
... and 58 more

🚀 New features to boost your workflow:

❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Deprecate postional arguments

8b73dbb

h-mayorquin marked this pull request as ready for review February 13, 2026 17:05

Merge branch 'main' into deprecate_postional_only_on_nwbfile

a6281e2

h-mayorquin mentioned this pull request Feb 13, 2026

Add test to enforce keyword arguments #1664

Merged

h-mayorquin self-assigned this Feb 13, 2026

h-mayorquin added the refactor label Feb 13, 2026

pauladkisson requested a review from Copilot February 13, 2026 17:48

Copilot started reviewing on behalf of pauladkisson February 13, 2026 17:49 View session

Copilot AI reviewed Feb 13, 2026

View reviewed changes

Copilot AI mentioned this pull request Feb 13, 2026

Fix unused stub_test parameter in OpenEphysBinaryRecordingInterface #1665

Closed

pauladkisson approved these changes Feb 13, 2026

View reviewed changes

Add test to enforce keyword arguments (#1664)

80ba461

h-mayorquin enabled auto-merge (squash) February 13, 2026 18:37

h-mayorquin merged commit c05185a into main Feb 13, 2026
24 checks passed

h-mayorquin deleted the deprecate_postional_only_on_nwbfile branch February 13, 2026 19:13

	super().__init__(filename=file_path, noise_std=noise_std)
	super().__init__(file_path=file_path)

Conversation

h-mayorquin commented Feb 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

h-mayorquin Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Feb 13, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI commented Feb 13, 2026

Uh oh!

pauladkisson left a comment

Choose a reason for hiding this comment

Uh oh!

h-mayorquin commented Feb 13, 2026

Uh oh!

Uh oh!

codecov bot commented Feb 13, 2026

Codecov Report

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants