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

Fix annotation #261

Merged
merged 12 commits into from
Jun 27, 2024
Merged

Fix annotation #261

merged 12 commits into from
Jun 27, 2024

Conversation

melonora
Copy link
Collaborator

@melonora melonora commented Jun 24, 2024

closes #260

This PR solves the issues mentioned in #260. The save message has been corrected and there are also no duplicate table names.
Regarding the latter point of adding new entries, saving and then going back to the old table leading to issues with non matching indices, I opted to delete the previous table. The other option would be to do a left exclusive join to see if there are indices not present in the table and then specifically add a row, but this seemed a bit more cumbersome. However this can be discussed.

We have a couple of cases for saving / exporting for element and table that are already stored:
1 shape name stays the same but table name is changed: in this case we can assume that new elements have been added making the old table invalid for the shape element. Thus the table with the old name is removed.
2 shape name is changed, but table name stays the same: this could be dealt with by a join but it is save to assume the user meant to delete the old shape element.
3 Both names are changed: this can be if there are diverging paths, no elements are deleted, new elements are just added.

Furthermore, if the spatialdata object is not backed, the elements are only added in memory and a notification is given that the spatialdata object is not backed and the elements are only added in memory.

Lastly, the first row with the default undefined class is now set not to be editable. The reason for this is that if someone has a shapes layer annotated with the default class, but then reopens it while having changed the default class, this results in an error.

@LucaMarconato
Copy link
Member

Fantastic! Looks great now. Also I see that the colors of the classes defined with the Annotation widget match with the colors displayed when plotting class using the View widget. Looks great!

I think it's ok to keep the currently implemented behavior regarding the last point.

Only a minor comment. For some reasons now after I draw a shape the shapes remain selected; so now if I change the class the latest drawn shape changes its color. I think it was more intuitive before when, after drawing the shape, the shape was not selected.

This behavior could be due to the fact that I am using some dev version of napari, so if you don't think this change is due to your PR, let's proceed with merging it!

@melonora
Copy link
Collaborator Author

The issue you mention seems to be mac specific, I don't have this at all. Then again with the napari 0.5.0 alpha release that just went down we might have to check this.

@melonora melonora merged commit e0085a8 into scverse:main Jun 27, 2024
1 of 6 checks passed
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.

Problems when saving multiple annotations tables for the same element
2 participants