Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

testing and fixing outdated atlas generation scripts #249

Merged
merged 8 commits into from
Mar 1, 2024

Conversation

viktorpm
Copy link
Contributor

@viktorpm viktorpm commented Feb 27, 2024

Description

We want to test if the atlas validation functions pass after regenerating atlases (see: #201).
Currently, the atlas generation scripts are outdated, so to regenerate the atlases, first, we need to fix and tidy the scripts.

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

We need an up-to-date version of the atlas generation scripts and the atlases.

What does this PR do?

Fixes syntax errors, and removes outdated packages.

References

#201

How has this PR been tested?

It's just a draft PR so far

Is this a breaking change?

It might be, due to changes in vedo syntax and function arguments.

Does this PR require an update to the documentation?

Yes

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

@viktorpm viktorpm self-assigned this Feb 27, 2024
@viktorpm
Copy link
Contributor Author

viktorpm commented Feb 27, 2024

Solved errors:

Script - error number Error Note Solved
admba_3d_dev_mouse.py - 1 Mesh.decimate() got an unexpected keyword argument 'method' replace with decimate_pro() Yes, tested locally & on HPC, commit: 0c1584a
allen_cord.py - 1 BadZipFile: File is not a zip file fixing source URL Yes, tested locally, commit: 5d4f4b0
allen_mouse.py no error - -
azba_zfish.py - 1 AttributeError: 'Mesh' object has no attribute 'smoothLaplacian' replace smoothLaplacian with smooth in mesh_utils.py Yes, tested on HPC, commit: a8f4631
example_mouse.py no error - -
kim_developmental_ccf_mouse.py - 1 AttributeError: 'DataFrame' object has no attribute 'append' possible pandas issue Yes, tested on HPC, commit: 58495b6
kim_mouse.py - 1 Mesh.decimate() got an unexpected keyword argument 'method' replace with decimate_pro() Yes, tested on HPC, creating meshes is very slow!
mpin_zfish.py - 1 allensdk pip install brainglobe-atlasapi[allenmouse] Yes
osten_mouse.py -1 allensdk pip install brainglobe-atlasapi[allenmouse] Yes, tested locally & on HPC
princeton_mouse.py no error - -
whs_sd_rat.py no error - -

Errors to be solved:

Script - error number Error Note Solved
admba_3d_dev_mouse.py - 2 gets stuck at region 112892417 of admba_3d_p28_mouse Creating mesh for region 112892417 No labels found for r10LimCG (112892417) no
admba_3d_dev_mouse.py - 3 doesn't skip already generated atlases FileExistsError: [Errno 17] File exists: no
humanatlas.py - 1 FileNotFoundError: No such file or no access: 'D:\Dropbox (UCL -SWC...' hardcoded paths in the code no, needs separate PR
mpin_zfish.py - 2 SSLError: HTTPSConnectionPool(...) - no
perens_lsfm_mouse.py - 1 IndexError: tuple index out of range no

@viktorpm
Copy link
Contributor Author

Steps to test changes on the HPC:

module load mamba
mamba init
mamba create -n bg-atlasapi python=3.11
mamba activate bg-atlasapi
git clone https://github.com/brainglobe/brainglobe-atlasapi.git
cd brainglobe-atlasapi/
git checkout fixing_atlas_scripts
pip install -e ".[allenmouse, dev]"
pip install scikit-image
pip install imio
pip install brainio
cd brainglobe_atlasapi/atlas_generation/atlas_scripts

@viktorpm viktorpm marked this pull request as ready for review February 29, 2024 15:17
Copy link
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.

Looking good - think we just need to fix the dependencies in pyproject.toml and then this is good to go 🚀

@@ -12,8 +12,11 @@
import numpy as np
import pandas as pd
from rich.progress import track

######## I had to manually install this
Copy link
Member

Choose a reason for hiding this comment

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

This should be added to the dependencies in pyproject.toml then, I think,

@@ -8,6 +8,8 @@
import treelib
import urllib3
from allensdk.core.structure_tree import StructureTree

### I had to install manually
Copy link
Member

Choose a reason for hiding this comment

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

Again, worth adding to the dependencies. If it is very big or hard-to-install, it can be an optional dependency like allensdk.

@@ -14,7 +14,10 @@

import os

######## I had to manually install this
Copy link
Member

Choose a reason for hiding this comment

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

We should depend on brainglobe-utils.imageio instead (see brainglobe/brainglobe-utils#33 for details), which may need to be added to the dependencies.

@alessandrofelder alessandrofelder merged commit 3c8a581 into main Mar 1, 2024
8 checks passed
@alessandrofelder alessandrofelder deleted the fixing_atlas_scripts branch March 1, 2024 16:52
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.

None yet

2 participants