Skip to content

Conversation

@FlynnOConnell
Copy link

A more general tiff interface that utilizes the new roiextractors.MultiTIFFMultiPageExtractor from catalystneuro/roiextractors#402

@h-mayorquin h-mayorquin marked this pull request as ready for review August 5, 2025 18:20
@h-mayorquin h-mayorquin marked this pull request as draft August 5, 2025 18:21
@codecov
Copy link

codecov bot commented Aug 5, 2025

Codecov Report

❌ Patch coverage is 61.90476% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 87.05%. Comparing base (8c0e9b0) to head (18b621d).

Files with missing lines Patch % Lines
...onv/datainterfaces/ophys/tiff/tiffdatainterface.py 61.90% 8 Missing ⚠️
Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1457      +/-   ##
==========================================
- Coverage   87.11%   87.05%   -0.06%     
==========================================
  Files         147      147              
  Lines        9746     9766      +20     
==========================================
+ Hits         8490     8502      +12     
- Misses       1256     1264       +8     
Flag Coverage Δ
unittests 87.05% <61.90%> (-0.06%) ⬇️

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

Files with missing lines Coverage Δ
...onv/datainterfaces/ophys/tiff/tiffdatainterface.py 77.14% <61.90%> (-22.86%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

num_acquisition_cycles=num_acquisition_cycles,
verbose=verbose,
)
self.imaging_extractor.get_num_channels = lambda: 1
Copy link
Collaborator

@h-mayorquin h-mayorquin Aug 14, 2025

Choose a reason for hiding this comment

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

We can remove this now that:

#1461

has been merged.

You can also point to the main branch of roiextractors pyproject.toml

Copy link
Author

Choose a reason for hiding this comment

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

You can also point to the main branch of roiextractors pyproject.toml

Not sure what you mean by this, the neuroconv pyproject points to the main branch already?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I was away last week so I could not come back to this:

I meant something like this:

image

But there is a released version of roiextractors that already has this feature so we don't need to do anything with the pyproject.toml anymore.

@h-mayorquin
Copy link
Collaborator

I was busy last week so now I am back on the trenches.

Are you still interested in moving forward with this PR?

If so, here's what we can do next:

  1. I'd like to merge this into the TiffImagingInterface, which should be straightforward. If you look at the signature, the only argument it currently has that this one doesn’t is file_path:

@validate_call
def __init__(
self,
file_path: FilePath,
sampling_frequency: float,
verbose: bool = False,
photon_series_type: Literal["OnePhotonSeries", "TwoPhotonSeries"] = "TwoPhotonSeries",
):
"""

We can deprecate the file_path argument in TiffImagingInterface and add the remaining arguments from this PR (including photon_series_type). Until file_path is fully removed, if someone passes it, we should internally convert it to file_paths within the relevant functions. Everything else should continue to work the same.

  1. We need to add more tests. You can reuse some of the structure and examples from the roiextractors module here:

https://github.com/catalystneuro/roiextractors/blob/cdfc407497810eb1b6d18d84e30ef60c7b6954d2/tests/test_multitiffmultipageextractor.py#L625-L639

These should go into:
neuroconv/tests/test_on_data/ophys/test_imaging_interfaces.py

  1. We should expand the conversion gallery to document the dimension_order setting:
    neuroconv/docs/conversion_examples_gallery/imaging/tiff.rst

I'm happy to take this to the finish line, but if you have the time and interest, it would be great to have you work on this. It's always valuable to have external contributors involved, though I would understand if you are busy.

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