Skip to content

add a class that allows storing locations of errors #46

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.

Sign up for GitHub

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

Open
moenigin wants to merge 10 commits into
base: master
Choose a base branch
from
Open

add a class that allows storing locations of errors #46

Changes from 1 commit
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
54a34dd
Update proofreading.py
moenigin Aug 15, 2023
6bc2857
pyink run on proofreading.py
moenigin Aug 21, 2023
94c7229
Merge remote-tracking branch 'upstream/master' into proofreading_updates
moenigin Aug 22, 2023
e5d48b9
Update proofreading.py
moenigin Aug 22, 2023
813ce6d
Update proofreading.py
moenigin Aug 22, 2023
b09792f
Update proofreading.py
moenigin Aug 22, 2023
d3cfa96
Update proofreading.py
moenigin Aug 23, 2023
5f7c55e
Update proofreading.py
moenigin Sep 12, 2023
45f2090
several fixes
moenigin Sep 13, 2023
448f18a
Merge remote-tracking branch 'upstream/master' into proofreading_updates
moenigin Sep 26, 2023
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
33 changes: 16 additions & 17 deletions ffn/utils/proofreading.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -367,7 +367,7 @@ def __init__(
seg_error_coordinates: A dictionary of error coordinates.
load_annotations: A flag to indicate if annotations should be loaded.
Copy link
Copy Markdown
Contributor

@mjanusz mjanusz Aug 22, 2023

Choose a reason for hiding this comment

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

What's a use case where you would provide seg_error_coordinates, but set load_annotations to False?

Copy link
Copy Markdown
Author

@moenigin moenigin Aug 22, 2023

Choose a reason for hiding this comment

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

A combination of laziness and generak flaws of the tool as is now. 1. If I pick up the review after a break I don't want to patch the final list of coordinates from different review session. 2. The annotation layer shows all annotations not only for the segment you are currently viewing. If in the last review session you already verfied that all locations you flagged in that session are correct, you may want to visualize only the novel locations.
Hope I am kind of making this clear here...

Copy link
Copy Markdown
Contributor

@mjanusz mjanusz Aug 22, 2023

Choose a reason for hiding this comment

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

OK, so is the idea that when load_annotations == False, the seg_error_coordinates are just stored (and appended to as you annotate more), but not actually displayed?

Copy link
Copy Markdown
Author

@moenigin moenigin Aug 23, 2023

Choose a reason for hiding this comment

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

Yes, that was my idea behind this because R. had already annotated >100 splits by the time we started this. Large numbers of annotations could be confusing. So if they are not needed because they are all verified from a previous revision round one can leave them out this way...
(Alternatively one could link the annotation to the errorneous segment of the todo list (requiring batch size to be one). By overriding update_segments one could only display the locations associated with a given batch of segments from the todo list. To view all marked location one could create a "show_all" function. However, as mentioned in the email thread I am not sure whether getting to split locations as implemented here is the most efficient way. Assembling segments belonging to one neurite by selection is usually faster. This would yield more coordinate pairs for a given split, but the would not necessarily be in close proximity to each other. If the latter is what you are after, that should be stated explicitly in the instructions.)

"""
super(ObjectReviewStoreLocation, self).__init__(objects, bad)
super().__init__(objects, bad)
self.seg_error_coordinates = seg_error_coordinates
if load_annotations and seg_error_coordinates:
for k, v in seg_error_coordinates.items():
Expand Down Expand Up @@ -459,19 +459,19 @@ def store_error_location(
self.temp_coord_list = []

def annotate_error_locations(
self, coordinate_lst: list[list[int]], id_: str
self, coordinates: list[list[int]], error_id: str
Copy link
Copy Markdown
Contributor

@mjanusz mjanusz Sep 26, 2023

Choose a reason for hiding this comment

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

Iterable[Point]?

) -> None:
"""Annotate the error locations in the viewer.

Args:
coordinate_lst: List of coordinates to be annotated.
id_: Unique identifier for the error.
coordinates: List of coordinates to be annotated.
error_id: Unique identifier for the error.
"""
for i, coord in enumerate(coordinate_lst):
annotation_id = id_ + f"_{i}"
self.mk_point_annotation(coord, annotation_id)
for i, coord in enumerate(coordinates):
annotation_id = f"{error_id}_{i}"
self.make_point_annotation(coord, annotation_id)

def mk_point_annotation(
def make_point_annotation(
self, coordinate: list[int], annotation_id: str
Copy link
Copy Markdown
Contributor

@mjanusz mjanusz Sep 26, 2023

Choose a reason for hiding this comment

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

Point?

) -> None:
"""Create a point annotation in the viewer.
Expand All @@ -488,8 +488,7 @@ def mk_point_annotation(
id=annotation_id, point=coordinate, props=[color]
)
with self.viewer.txn() as s:
annotations = s.layers["annotation"].annotations
annotations.append(annotation)
s.layers["annotation"].annotations.append(annotation)

def get_annotation_id(
self, action_state: neuroglancer.viewer_config_state.ActionState
Expand Down Expand Up @@ -519,17 +518,17 @@ def delete_location_from_annotation(
Args:
action_state: State of the viewer during the action.
"""
id_ = self.get_annotation_id(action_state)
if id_ is None:
ann_id = self.get_annotation_id(action_state)
if ann_id is None:
return

target_key = id_[:2]
target_key, _ = ann_id.split("_")
del self.seg_error_coordinates[target_key]

to_remove = [target_key + "_0", target_key + "_1"]
to_remove = frozenset([target_key + "_0", target_key + "_1"])
self.delete_annotation(to_remove)

def delete_annotation(self, to_remove: list[str]) -> None:
def delete_annotation(self, to_remove: frozenset[str]) -> None:
"""Delete specified annotations from the viewer.

Args:
Expand All @@ -540,9 +539,9 @@ def delete_annotation(self, to_remove: list[str]) -> None:
annotations = [a for a in annotations if a.id not in to_remove]
s.layers["annotation"].annotations = annotations

def delete_last_location(self):
def delete_last_location(self) -> None:
"""Delete the last error location pair tagged."""
last_key = list(self.seg_error_coordinates.keys())[-1]
last_key = next(reversed(self.seg_error_coordinates))
del self.seg_error_coordinates[last_key]

to_remove = [last_key + "_0", last_key + "_1"]
Comment thread
mjanusz marked this conversation as resolved.
Outdated
Expand Down