Conversation
Added information about brainglobe atlas API names and their conversions.
…rainglobe/brainglobe-ccf-translator into auto-detect-same-space-atlases
|
once #23 is merged Ill mark this ready for review |
IgorTatarnikov
left a comment
There was a problem hiding this comment.
LGTM 🎉
It would be good to expand the docstrings in any public function.
Is the plan to switch from unitttest to pytest for testing? Most of our CI machinery is built around the assumption that pytest is being used as the underlying framework.
There was a problem hiding this comment.
Could you expand the docstrings a bit?
Some of the public functions are missing a description for their parameters, and the return value.
There was a problem hiding this comment.
Just flagging this, the other files only had white space changes while here the value changed. I'm assuming this matches with changes in the functionality?
* include demba api name in autoconvert list * update metadata and ingestion scripts to include dorr * update readme * run graph update script and update readme * run graph update script and update readme * update dorr age to p84 * update accessibility text as we are no longer using an image * update ruff formatting * run precommit * precommit * replace click with print * add mode arg to suppress future warning * ruff reformat * unittest worked but pytest failed, here we introduce a fix for this * drop workflows for 3.9 and 3.10 as they seem to run out of resources * use atlas api to access atlas shapes * remove license to make pre-commit happy * Remove unnecessary whitespace in config.py
* remove testing file * auto publish to pypi on release * update yml file and python versions * remove scheduled deploy * remove twine api key as we can instead set the repo to be a trusted publisher for token less pushing * run black linter * enforce line lengths * enforce 79 char line length * exclude files that will not be included in pypi * remove line length from ruff as its failing test (its still specified elsewhere) * fix issue with test * update license file to avoid spdx warning * ruff fixes * correct line incorrectly removed from ingestion script and caught by precommit * remove unused variable caught by precommit * make ruff happy * ruff reformat * exclude utilities * precommit passed locally but failed remotely, updating reqs and license * move docstring to beginning of file * tests failing bc of triple quoted comments * ruff reformat * test off by 0.5 voxels... * rename test case * revert test value * fix empty last line * autofix files precommit * Auto detect same space atlases (#26) * implement atlas synonyms to be more compatible with brainglobe * return atlas synonyms among near misses * Update README with atlas synonyms Added information about brainglobe atlas API names and their conversions. * precommit * precommit and add documentation about running just one test * add to ignore words list so that codespell doesnt think the method name is a typo * include demba api name in autoconvert list * update metadata and ingestion scripts to include dorr * update readme * run graph update script and update readme * run graph update script and update readme * update dorr age to p84 * update accessibility text as we are no longer using an image * update ruff formatting * run precommit * precommit * replace click with print * add mode arg to suppress future warning * ruff reformat * unittest worked but pytest failed, here we introduce a fix for this * drop workflows for 3.9 and 3.10 as they seem to run out of resources * use atlas api to access atlas shapes * remove license to make pre-commit happy * Dorr mri (#27) * include demba api name in autoconvert list * update metadata and ingestion scripts to include dorr * update readme * run graph update script and update readme * run graph update script and update readme * update dorr age to p84 * update accessibility text as we are no longer using an image * update ruff formatting * run precommit * precommit * replace click with print * add mode arg to suppress future warning * ruff reformat * unittest worked but pytest failed, here we introduce a fix for this * drop workflows for 3.9 and 3.10 as they seem to run out of resources * use atlas api to access atlas shapes * remove license to make pre-commit happy * Remove unnecessary whitespace in config.py --------- Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com> --------- Co-authored-by: Igor Tatarnikov <61896994+IgorTatarnikov@users.noreply.github.com>
adamltyson
left a comment
There was a problem hiding this comment.
Hey @PolarBean, I wrote this comment 2
months ago, but forgot to submit it. Probably still relevant now though.
| | Gubra MRI mouse | perens_stereotaxic_mri_mouse| 56 | ||
| | Princeton lightsheet mouse | princeton_mouse| 56 | ||
|
|
||
| We also support brainglobe atlas api names which are in existing coordinate frameworks. For instance you can specify osten_mouse and CCF translator will autoconvert this to allen_mouse. |
There was a problem hiding this comment.
Tiny comment, but osten_mouse is an atlas that was added for an internal project. I don't think there's much reason for anyone else to use it. Might be worth using a different example so we don't accidently suggest to others to use a less than ideal atlas.
Description
This PR allows users to specify other brainglobe atlas names that are in existing spaces. ccf translator will then automatically convert these names to the already existing space. Currently we are adding kim_mouse, allen_mouse_bluebrain_barrels, and osten_mouse which are all in allen space.
we also add a check for near misses of space names using the builtin strandard python library difflib. This checks for things like allenk_nmouse and suggests to the user that they may have meant allen_mouse. This is especially useful as it catches if people say things like allen_mouse_25um and suggests that they may have meant allen_mouse.
What is this PR
How has this PR been tested?
we add several new tests for these functions. ensuring that the result using a synonym is the same as using the main atlas name
Does this PR require an update to the documentation?
I will update the readme to include the synonyms
Checklist: