Closed willGraham01 closed 10 months ago
Looks good @willGraham01, thanks! Two small comments:
New ethos is that this package is just a fast one-time-quick install, so we don't directly expose anything under this package
Did we have this discussion? There's no reason for it to be in the initial release, but I think we should work towards from brainglobe import cellfinder
etc.
When restructuring brainreg, I think we should also seperate the CLI for consistency. I don't think it needs to go into brainglobe-workflows
as the CLI only includes functionality from the brainreg
API. See https://github.com/brainglobe/brainreg/issues/139.
Did we have this discussion? There's no reason for it to be in the initial release, but I think we should work towards from brainglobe import cellfinder etc.
You're right sorry - I've mixed this up with brainglobe-workflows
(which we agreed should be standalone-installable without fetching cellfinder
& brainreg
). brainglobe-workflows
will still ship with brainglobe-meta
so will still be exposed via CLI, but of course doesn't need to be exposed via brainglobe
(unless we want a from brainglobe import workflows
-style API too)?
When restructuring brainreg, I think we should also seperate the CLI for consistency. I don't think it needs to go into brainglobe-workflows as the CLI only includes functionality from the brainreg API. See https://github.com/brainglobe/brainreg/issues/139.
I've updated the brainreg
task to make sure this is included.
You're right sorry - I've mixed this up with brainglobe-workflows (which we agreed should be standalone-installable without fetching cellfinder & brainreg). brainglobe-workflows will still ship with brainglobe-meta so will still be exposed via CLI, but of course doesn't need to be exposed via brainglobe (unless we want a from brainglobe import workflows-style API too)?
For now at least, I think workflows can remain CLI only. I would think that if users are using the API, they will be creating their own workflow, not importing an existing, inflexible one.
@adamltyson @alessandrofelder We previously said that workflows
should not fetch cellfinder
nor brainreg
, but should rely on users having them already. Not sure what our ethos was, but I'm guessing it was along the lines of when the meta-package is ready this won't matter.
However since then, it looks like workflows
is now depending on cellfinder-core
anyway. There's no harm in having them as fixed dependencies, and since we're doing the BrainGlobe v1 update incrementally, should I stick to this convention?
Could we directly depend on brainglobe-meta already? If not, whatever is easiest - seems like something that can be easily tidied up after?
Could we directly depend on brainglobe-meta already?
Maybe - I'm not sure what that would mean when for when we merge cf-core
and cf-napari
and rename them cellfinder
though. But yeah, once all the packages are moved around we can clean up later.
I thought the plan is to have brainglobe-meta
as the top-level package though? So brainglobe-meta
should be fetching brainglobe-workflows
(since users will still want to use the cellfinder
CLI tool). workflows
technically would need to depend on the other brainglobe packages in case anyone wanted to install it standalone. If installing the meta-package, it'll fetch workflows
and all its dependencies anyway so users don't need to worry.
I thought the plan is to have brainglobe-meta as the top-level package though? So brainglobe-meta should be fetching brainglobe-workflows (since users will still want to use the cellfinder CLI tool).
Right, sorry, makes sense.
OK, so IIUC at least for our CI we should have it depend on the other brainglobe packages - so we may as well make it depend on the ones it needs, for now?
so we may as well make it depend on the ones it needs, for now?
Sounds good. We can always "hide" them as dev/benchmark
dependencies later if there's anything we really don't want users to get mixed up in.
I think the idea was for workflows to depend on bg-meta, but not the other way round. This would make sure the workflows (which are not ideal in their structure) don't break the metapackage.
Plus workflows can/will contain non-BG dependencies that may be incompatible.
Plus workflows can/will contain non-BG dependencies that may be incompatible.
Good point. So for now
cellfinder-core
and whatever else we need (e.g to run the cellfinder benchmark)?
Sounds good to me.
BrainGlobe v1 Release Plan (currently in writing)
Essentially a more coherent summary of this plan, however updated with the decisions taken from the 31/08/2023 developer meeting.
UPDATE: Link is now broken, but the full plan is in the task list below.
Overview
For the release of BrainGlobe version 1, we plan to restructure a number of packages, rename/move a series of tools, reduce the number of separate tools that we bundle and require install for, and update the documentation to be consistent with these changes.
At a high level, these changes are:
brainglobe-workflows
repository, and moving the CLI tools currently in thecellfinder
repository there, and renaming them.cellfinder-core
andcellfinder-napari
tocellfinder
(this is a fundamental change in the package purpose).brainreg
,brainreg-segment
, andbrainreg-napari
packages together in similar manner tocellfinder-napari
andcellfinder-core
.brainglobe-registration
,brainreg-segment
will remain standalone and becomebrainglobe-segmentation
.brainglobe-meta
package to provide a one-linepip
install with the new package structure.Renaming on GitHub and PyPI knock-on effects
We want to retain the GitHub repositories for their usage metrics and historial links provided in publications. We also cannot retire names on PyPI and then re-upload a package with the same name and different functionality. Therefore, our plan is to sequentially "move" functionality between repositories on GitHub (which will preserve the usage metrics), through a series of PRs and the repository merge trick.
For PyPI, we will release "final" versions for all packages that are becoming redundant with no homonymous replacement package. These final versions will introduce no functionality changes from the previous version, but the READMEs will update to point to the replacement package. These versions should all be tagged with a suitable version number, ideally a version less than 1.0 to signify they were never intended for release. They can then be marked as deprecated on PyPI.
For packages whose functionality is being moved and then replaced (
cellfinder
and possiblybrainreg
), we will do a similar thing to the above for the old state of the package - push a new version to PyPI pointing to the replacement. Then when the new functionality replaces the old in the GitHub repo, we will push a new version release (v1.0 or higher) to PyPI with the new functionality. This will have the slightly awkward affect of muddling our PyPI release history, but this is something we should comfortably be able to manage ourselves through the meta-package dependencies.Detailed Task List
Tasks are organised into groups based on the packages they affect. These groups are presented in a roughly chronological order, however due to the interdependencies between packages there will inevitably be blockages at certain points.
Although it's explicitly mentioned in each of the task lists below, these changes should all be done against a development branch in each repository. Then the plan will be for us to pick a day, merge all the dev PRs into the respective mains, and then push new PyPI uploads. This should ensure that none of our packages are left broken, but it also means that any hotfixes can be incorporated into the dev-branches via a rebase.
To track the completion of tasks, once a PR (against a dev branch or otherwise) is opened, add a link to that PR against the corresponding task in this list. When the PR is merged, we can then tick off the sub-tasks and see where the change was introduced.
brainglobe/brainglobe-workflows
brainglobe
meta-package rather than individual tools - https://github.com/brainglobe/brainglobe-workflows/pull/72brainglobe/cellfinder
cellfinder-core
andcellfinder-napari
#46brainglobe/cellfinder-core
brainglobe/cellfinder-napari
brainglobe/brainreg-segment
brainglobe/brainreg-napari
brainglobe/brainreg
brainglobe/brainglobe-meta
cellfinder
, pinned>=1.0
brainreg
, pinned>=1.0
brainreg
,cellfinder
, etc tools under brainglobe namebrainglobe/website
(and other documentation related tasks)pip
into a cleanvenv
/conda env
etc.napari
, which will require some additional instructionsbrainglobe-workflows
overview page, #45cellfinder
used to do, and has now been moved herecellfinder
updates, #45cellfinder-core
updates #46cellfinder-napari
updates #46brainreg
updates #39