brainglobe / brainglobe-workflows

Workflows that utilise BrainGlobe tools to perform data analysis and visualisation.
BSD 3-Clause "New" or "Revised" License
9 stars 2 forks source link

Coexistance of cellfinder+brainreg CLI tool and cellfinder-core workflow #58

Closed sfmig closed 11 months ago

sfmig commented 11 months ago

Context

This repository currently serves a few purposes:

Our understanding is that the cellfinder+brainreg CLI tool will be gradually discontinued (in line with the brainglobe general aim of removing CLI tools) and turned into a workflow script instead. This PR outlines a strategy to get there.

Goal of this PR

With this PR, the brainglobe-workflows PyPI package will ship:

The cellfinder_core workflow script is only included in the source code, and users are not meant to be "aware" of it (aka, it is not documented in the user docs). Users will use the cellfinder+brainreg CLI tool in the same way as they did before.

Instead, developers are aware of the cellfinder_core workflow script and of the benchmarks defined on top of it (that is, both things are documented in the dev docs).

How do users use this repo?

Users will pip install brainglobe-workflows and use the cellfinder+brainreg CLI tool as usual, with the cellfinder entrypoint.

How do devs use this repo?

Devs will:

  1. git clone this repo
  2. pip install .[dev]
  3. Run benchmarks locally with asv run or run tests with tox/pytest

Three sets of dependencies

To cater for these different audiences, we divided the dependencies in pyproject.toml into three sets:

Longer term

Specifics of this PR

Rebase after merging #27

adamltyson commented 11 months ago

I might have missed something, but is this needed? What used to be called cellfinder-core is now called cellfinder anyway now.

alessandrofelder commented 11 months ago

We need to somehow distinguish the CLI tool (present in this repo) from benchmarked scripts using the Python tool's API?

sfmig commented 11 months ago

my understanding is that the cellfinder+brainreg CLI tool is called in this repo cellfinder after the migration of the cellfinder repo to the brainglobe-workflows repo. This is in conflict with me previously using cellfinder to define a cellfinder-core workflow.

adamltyson commented 11 months ago

my understanding is that the cellfinder+brainreg CLI tool is called in this repo cellfinder after the migration of the cellfinder repo to the brainglobe-workflows repo. This is in conflict with me previously using cellfinder to define a cellfinder-core workflow.

This should change soon anyway, but maybe it's good to specify cellfinder_core to distinguish from cellfinder.napari anyway.

adamltyson commented 11 months ago

In the longer term, the cellfinder+brainreg CLI tool will be transformed into a workflow script

Slight comment, I think we will maintain some CLIs (i.e. just the command line interface) that are heavily used. However, I think we should move most of the code inside these workflows to other packages so that it can be a single workflow script.

codecov[bot] commented 11 months ago

Codecov Report

Attention: 2 lines in your changes are missing coverage. Please review.

Comparison is base (24f2592) 81.37% compared to head (9c19237) 81.39%.

Files Patch % Lines
benchmarks/cellfinder_core.py 0.00% 2 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #58 +/- ## ========================================== + Coverage 81.37% 81.39% +0.02% ========================================== Files 32 32 Lines 1584 1586 +2 ========================================== + Hits 1289 1291 +2 Misses 295 295 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.