Skip to content
This repository has been archived by the owner on Feb 23, 2024. It is now read-only.

printing data frames with validation function information #115

Merged
merged 23 commits into from
Feb 12, 2024
Merged
Changes from 14 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
d6d481b
first test functions for validate_mesh_structure_pairs
viktorpm Jan 29, 2024
512cd63
storing atlases and successful/failed validation functions in a data …
viktorpm Jan 31, 2024
3aaf90a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Jan 31, 2024
4e29467
restoring test_validation.py to the original merged version. Chages a…
viktorpm Jan 31, 2024
84de0d1
restoring test_validation.py to the original merged version. Chages a…
viktorpm Jan 31, 2024
f6b0dc0
validate_atlases.py: going back to the version on main, appending onl…
viktorpm Feb 1, 2024
fd8a6a8
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 1, 2024
a0ba87b
populating dictionaries in for loop, writing JSON files
viktorpm Feb 1, 2024
6d9707b
Merge branch 'cosmetics' of github.com:brainglobe/bg-atlasgen into co…
viktorpm Feb 1, 2024
0bd3163
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 1, 2024
a980f67
saving JSON files to ~/.brainglobe/atlases/validation
viktorpm Feb 2, 2024
eec486a
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 2, 2024
398dccc
printing where to find the result files
viktorpm Feb 2, 2024
56686e1
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 2, 2024
576d8d2
Update bg_atlasgen/validate_atlases.py
viktorpm Feb 5, 2024
d7a7d3b
Update bg_atlasgen/validate_atlases.py
viktorpm Feb 5, 2024
defaa29
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 5, 2024
16dee78
Merge branch 'main' into cosmetics
viktorpm Feb 6, 2024
8773286
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 6, 2024
da4cc8f
saving only one JSON file with all the information
viktorpm Feb 9, 2024
6c00f6d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Feb 9, 2024
58722fd
uncommenting test functions
viktorpm Feb 9, 2024
dea87e4
Merge branch 'cosmetics' of github.com:brainglobe/bg-atlasgen into co…
viktorpm Feb 9, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 47 additions & 9 deletions bg_atlasgen/validate_atlases.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"""Script to validate atlases"""


import json

Check warning on line 4 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L4

Added line #L4 was not covered by tests
import os
from pathlib import Path

Expand All @@ -15,10 +16,10 @@
from bg_atlasapi.update_atlases import update_atlas


def validate_atlas_files(atlas: BrainGlobeAtlas):

Check warning on line 19 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L19

Added line #L19 was not covered by tests
"""Checks if basic files exist in the atlas folder"""

atlas_path = (

Check warning on line 22 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L22

Added line #L22 was not covered by tests
Path(get_brainglobe_dir())
/ f"{atlas.atlas_name}_v{get_local_atlas_version(atlas.atlas_name)}"
)
Expand Down Expand Up @@ -95,26 +96,26 @@
return True


def open_for_visual_check(atlas: BrainGlobeAtlas):

Check warning on line 99 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L99

Added line #L99 was not covered by tests
# implement visual checks later
pass


def validate_checksum(atlas: BrainGlobeAtlas):

Check warning on line 104 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L104

Added line #L104 was not covered by tests
# implement later
pass


def check_additional_references(atlas: BrainGlobeAtlas):

Check warning on line 109 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L109

Added line #L109 was not covered by tests
# check additional references are different, but have same dimensions
pass


def validate_mesh_structure_pairs(atlas: BrainGlobeAtlas):

Check warning on line 114 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L114

Added line #L114 was not covered by tests
"""Ensure mesh files (.obj) exist for each expected structure in the atlas."""
ids_from_bg_atlas_api = list(atlas.structures.keys())

Check warning on line 116 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L116

Added line #L116 was not covered by tests

atlas_path = (

Check warning on line 118 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L118

Added line #L118 was not covered by tests
Path(get_brainglobe_dir())
/ f"{atlas.atlas_name}_v{get_local_atlas_version(atlas.atlas_name)}"
)
Expand Down Expand Up @@ -143,7 +144,7 @@
)


def validate_atlas(atlas_name, version, validation_functions):

Check warning on line 147 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L147

Added line #L147 was not covered by tests
"""Validates the latest version of a given atlas"""

print(atlas_name, version)
Expand All @@ -152,16 +153,20 @@
if not updated:
update_atlas(atlas_name)

# list to store the errors of the failed validations
# dictionary of lists to store the errors of the failed validations
failed_validations = {atlas_name: []}
successful_validations = {atlas_name: []}

Check warning on line 158 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L157-L158

Added lines #L157 - L158 were not covered by tests

for i, validation_function in enumerate(validation_functions):

Check warning on line 160 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L160

Added line #L160 was not covered by tests
try:
validation_function(BrainGlobeAtlas(atlas_name))
successful_validations[atlas_name].append(validation_function)
successful_validations[atlas_name].append(

Check warning on line 163 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L162-L163

Added lines #L162 - L163 were not covered by tests
validation_function.__name__
)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
successful_validations[atlas_name].append(
validation_function.__name__
)
validations[atlas_name].append(
validation_function.__name__, None
)

(:arrow_up: Just a sketch... and just a discussion point)

Should we combine the two lists while we're refactoring this, and have them have the same format (str(error) if failed, and None if valid)? The advantage would be simplicity

  • just one file
  • shorter and easier to read code

But it's entirely possible there's value in keeping the things separate? What do you think?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Thanks for this suggestion! I was also thinking about doing this but wanted to have a chat first, as I wasn't sure how to implement it. I think it would be much better to have only one file with all the necessary information.

Copy link
Collaborator Author

@viktorpm viktorpm Feb 9, 2024

Choose a reason for hiding this comment

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

@alessandrofelder, I implemented these changes and tested them locally and on the HPC. Could you check them, please? 🙂
I think it's ready to be merged if you approve the changes.

result file:
validation_results.json

Copy link
Member

Choose a reason for hiding this comment

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

Thanks @viktorpm - As per our developer's guide around PRs, you have the liberty to merge if your reviewer approves with optional comments without asking for another round of review :) I have had another look though and looks great!

except AssertionError as error:
failed_validations[atlas_name].append((validation_function, error))
failed_validations[atlas_name].append(

Check warning on line 167 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L167

Added line #L167 was not covered by tests
(validation_function.__name__, str(error))
)

return successful_validations, failed_validations

Expand All @@ -179,17 +184,50 @@

valid_atlases = []
invalid_atlases = []
successful_validations = {}
failed_validations = {}

Check warning on line 188 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L187-L188

Added lines #L187 - L188 were not covered by tests

for atlas_name, version in get_all_atlases_lastversions().items():
successful_validations, failed_validations = validate_atlas(
temp_successful_validations, temp_failed_validations = validate_atlas(

Check warning on line 191 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L191

Added line #L191 was not covered by tests
atlas_name, version, all_validation_functions
)

successful_validations.update(temp_successful_validations)
failed_validations.update(temp_failed_validations)

Check warning on line 196 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L195-L196

Added lines #L195 - L196 were not covered by tests

for item in successful_validations:
valid_atlases.append(item)
for item in failed_validations:
invalid_atlases.append(item)
viktorpm marked this conversation as resolved.
Show resolved Hide resolved

print("Summary")
print("### Valid atlases ###")
print(valid_atlases)
print("### Invalid atlases ###")
print(invalid_atlases)
print("Validation has been completed")
print(

Check warning on line 204 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L203-L204

Added lines #L203 - L204 were not covered by tests
"Find successful_validations.json and failed_validations.json in ~/.brainglobe/atlases/validation/"
)

# Get the directory path
output_dir_path = str(get_brainglobe_dir() / "atlases/validation")

Check warning on line 209 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L209

Added line #L209 was not covered by tests

# Create the directory if it doesn't exist
if not os.path.exists(output_dir_path):
os.makedirs(output_dir_path)

Check warning on line 213 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L212-L213

Added lines #L212 - L213 were not covered by tests

# Open a file for writing
viktorpm marked this conversation as resolved.
Show resolved Hide resolved
with open(

Check warning on line 216 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L216

Added line #L216 was not covered by tests
str(
get_brainglobe_dir() / "atlases/validation/failed_validations.json"
),
"w",
) as file:
# Write the dictionary to the file in JSON format
json.dump(failed_validations, file)

Check warning on line 223 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L223

Added line #L223 was not covered by tests

with open(

Check warning on line 225 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L225

Added line #L225 was not covered by tests
str(
get_brainglobe_dir()
/ "atlases/validation/successful_validations.json"
),
"w",
) as file:
# Write the dictionary to the file in JSON format
json.dump(successful_validations, file)

Check warning on line 233 in bg_atlasgen/validate_atlases.py

View check run for this annotation

Codecov / codecov/patch

bg_atlasgen/validate_atlases.py#L233

Added line #L233 was not covered by tests
Loading