Skip to content

MACSIMA parse subfolders only when explicitly requested#379

Merged
LucaMarconato merged 7 commits intoscverse:mainfrom
MSHelm:feature/macsima_parse_subfolders_only_when_explicitly_requested
May 7, 2026
Merged

MACSIMA parse subfolders only when explicitly requested#379
LucaMarconato merged 7 commits intoscverse:mainfrom
MSHelm:feature/macsima_parse_subfolders_only_when_explicitly_requested

Conversation

@MSHelm
Copy link
Copy Markdown
Contributor

@MSHelm MSHelm commented Feb 27, 2026

This implements the proposed change of macsima parsing style options, described in #378

Remove auto parsing style option entirely.
By default, all tif files inside a folder and all its subdirectories are parsed into a single Image element inside a SpatialData object.
Only if the user specifically requests parsing subfolders, then the first level underneath the specified directory defines the image elements. All tifs in these directories, and subdirectores, will be parsed into separate image elements, with corresponding tables and coordinate systems.
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Feb 27, 2026

Codecov Report

❌ Patch coverage is 60.00000% with 2 lines in your changes missing coverage. Please review.
✅ Project coverage is 62.92%. Comparing base (fafeac8) to head (6659f99).

Files with missing lines Patch % Lines
src/spatialdata_io/readers/macsima.py 60.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #379      +/-   ##
==========================================
+ Coverage   62.77%   62.92%   +0.15%     
==========================================
  Files          26       26              
  Lines        3175     3172       -3     
==========================================
+ Hits         1993     1996       +3     
+ Misses       1182     1176       -6     
Files with missing lines Coverage Δ
src/spatialdata_io/__main__.py 84.54% <ø> (ø)
src/spatialdata_io/readers/macsima.py 95.27% <60.00%> (+1.61%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@LucaMarconato
Copy link
Copy Markdown
Member

@MSHelm looks good to me! We can merge after #371 is merged.

@MSHelm
Copy link
Copy Markdown
Contributor Author

MSHelm commented May 4, 2026

@LucaMarconato Great, thanks a lot. So far I haven't added tests for this PoC implementation, I will do this tomorrow.

@LucaMarconato
Copy link
Copy Markdown
Member

Thanks for adding tests! I'll review.

Comment thread tests/test_macsima.py
_parse_v1_ome_metadata,
macsima,
)
from tests._utils import skip_if_below_python_version
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good point! No need for the skip_if_below_python_version machinery, since you have

if not (Path("./data/OMAP10_small").exists() or Path("./data/OMAP23_small").exists()):
    pytest.skip()

And in the test.yaml workflow we already skip downloading the artifacts for python < 3.13

Copy link
Copy Markdown
Member

@LucaMarconato LucaMarconato left a comment

Choose a reason for hiding this comment

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

Looks great to me! I have just made some minor modifications, which make the pytest checks more explicits.

@LucaMarconato
Copy link
Copy Markdown
Member

We can merge after #371 is merged.

We can actually merge already!

@LucaMarconato
Copy link
Copy Markdown
Member

Tests fail due to this line, which comes from main with c8f6191

parsing_style: str = "auto",

@LucaMarconato LucaMarconato merged commit 5c43217 into scverse:main May 7, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants