Skip to content

fix: allow detection-only runs without background channel#610

Open
NiShITa-code wants to merge 1 commit into
brainglobe:mainfrom
NiShITa-code:feat/fix-605-optional-background-skip-classification
Open

fix: allow detection-only runs without background channel#610
NiShITa-code wants to merge 1 commit into
brainglobe:mainfrom
NiShITa-code:feat/fix-605-optional-background-skip-classification

Conversation

@NiShITa-code
Copy link
Copy Markdown

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

cellfinder.core.main.main(...) currently requires a background_array even when skip_classification=True.

That blocks single-channel detection-only use cases and causes the Python API to fail before the skip-classification logic is reached. This is also a concrete blocker related to optional-background support discussed in #352.

What does this PR do?

  • makes background_array optional at the cellfinder.core.main.main(...) entrypoint
  • allows detection-only runs when skip_classification=True and no background channel is provided
  • preserves the requirement for background_array when classification is enabled
  • adds regression tests for both the allowed detection-only case and the classification-required error case

References

How has this PR been tested?

I added regression tests covering:

  • detection-only execution without background_array
  • raising a clear error when classification is requested without background_array

I also manually reviewed the changed code paths to ensure classification still requires background input.

Is this a breaking change?

No.

This change preserves existing behavior for classification workflows and only relaxes the API for detection-only runs when skip_classification=True.

Does this PR require an update to the documentation?

No documentation changes were made in this PR.

This is a small bug fix to the Python API behavior. If preferred, I’m happy to add a short note to the API docs in a follow-up PR.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality (unit & integration)
  • The documentation has been updated to reflect any changes
  • The code has been formatted with pre-commit

Copy link
Copy Markdown
Member

@alessandrofelder alessandrofelder left a comment

Choose a reason for hiding this comment

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

Thanks for contributing @NiShITa-code and sorry it took so long to get back to you.

I understand your approach, but I don't think it's clean enough - have explained in a comment.

I suggest instead that we keep the arguments, but allow None to be explicitly passed for the background array, rather than being the default.

Comment thread cellfinder/core/main.py

# Preserve the historical positional API while allowing callers to omit
# background data for detection-only runs.
if voxel_sizes is None:
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.

I don't think this is a clean way to retain the API, because while the positional API is maintained, you could pass the voxel sizes via the background array keyword argument, and these are very different things!

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.

[BUG]: background channel is required even when skip_classification=True (single-channel flow breaks)

2 participants