[Feature] make cellfinder main functions' arguments keyword-only

[alessandrofelder]

Is your feature request related to a problem? Please describe.
As first suggested by @matham in #542 (comment), calling the various "main" detect+classify functions, which have many arguments, with the arguments in the right order, is error-prone.

Describe the solution you'd like
Make the main functions' arguments keyword-only

[alessandrofelder]

I am generally supportive of this. My only reservation would be if this is likely to cause maintainers of other libraries lots of (short-term) pain, but I don't think this is case for cellfinder?

[adamltyson]

Also supportive. It should be an easy enough fix for others.

[Chiraagkv]

Hi! If this issue is still valid, I’d like to work on it as a first contribution.

My understanding is that the goal is to make the arguments of the main functions of detect and classify keyword-only to reduce errors when calling them with many parameters. My plan would be to update the function signatures and adjust any internal calls accordingly.

Please let me know if that approach sounds reasonable.

[adamltyson]

This sounds reasonable. However please note that this work will also include:

• deprecating the old syntax
• updating other BrainGlobe repos to use the new syntax
• checking to see if any other repos in the community would break following this change and raising issues as appropriate.

[Haris-bin-shakeel]

Hi, I'd like to take this on if it's still available.

I've gone through the codebase and have a clear picture of the scope.
A few things I noticed that are relevant to @adamltyson's March 9 note:

Internal callers to update:

• cellfinder/core/main.py:240, calls classify.main(...) with positional
  arguments and will need updating
• cellfinder/core/main.py:205 , calls detect.main(...) already using
  keywords, so no change needed there
• cellfinder/napari/detect/thread_worker.py:89, already uses **kwargs
  passthrough, safe

Tests to update:

• tests/core/test_unit/test_main.py
• tests/core/test_integration/test_detection.py (e.g. line 79 uses
  positional args)
• tests/core/test_unit/test_detect/test_detect.py

Deprecation approach:
I'll follow the warnings.warn(..., DeprecationWarning, stacklevel=2)
pattern rather than abrupt breakage, to give downstream users a grace period.

External repos:
I'll check brainglobe-workflows (brainmapper CLI) and raise issues
there if positional calls are found, as suggested.

Happy to get a green light before opening the PR. Is there anything
specific about the deprecation timeline or grace period I should know before
starting?

[adamltyson]

Hi @Haris-bin-shakeel, yes it's still available. As per my comment, we should check for other dependent repos too (outside of the org). I don't think there are many.

Feel free to open a PR. Cellfinder moves relatively slowly, so I would say a 6-12 month grace period? @alessandrofelder any thoughts?