add option to overwrite a previous atlas import

[carlocastoldi]

With this PR:

• the Root annotation is now classified by the corresponding atlas version name
• the Root annotation is locked
• if a previous atlas import was modified but its hierarchy is not disrupted (i.e. everything is still a child of Root), it can be re-imported without resulting in duplicate annotations
• removes the atlasName from some API calls when the onthology is passed too. May be breaking, but it is redundant and misleading
• fixes some deprecated calls
• renames an example script

[carlocastoldi]

a couple of additional questions:

• is there a particular reason to why loadWarpedAtlasAnnotations() does not return the Root annotation?
• should we fix the tests so that they're actually useful?
• do you think we should make LoadAtlasRoisToQuPathCommand be a little be more complex?

[NicoKiaru]

Hi @carlocastoldi ,

Thanks for the PR. Just a few changes:

• I think we can keep the old API and mark it as deprecated while implementing the new API.
• I'm not sure what overwrite does
• Please keep the def keywords. That's for consistency with Fiji behavior

[NicoKiaru]

Too much fancyness! I prefer the simple notation

[NicoKiaru]

a couple of additional questions:

* is there a particular reason to why `loadWarpedAtlasAnnotations()` does not return the Root annotation?

Not really, I think it could return the root annotation without any problem.

* should we fix the tests so that they're actually useful?

Sure, if you know how to do that, that'd be nice.

* do you think we should make `LoadAtlasRoisToQuPathCommand` be a little be more complex?

Hmm, why not. But this one is simple, and people like simplicity. Why not adding another one if you need more fine tuning. But more importantly, what do you have in mind in terms of complexity?

Thanks for the PR!

[NicoKiaru]

All good, thanks!

[NicoKiaru]

So, here's the commit message:

• loading annotations can now overwrite any previous previous import of the same atlas version. Overwrite will only work if the previous atlas's hierarchy wasn't disrupted
• fixed deprecated calls
• the Root annotation is now classified by the corresponding atlas version name
• the Root annotation is locked
• if a previous atlas import was modified but its hierarchy is not disrupted (i.e. everything is still a child of Root), it can be re-imported without resulting in duplicate annotations
• removes the atlasName from some API calls when the onthology is passed too. The old API is deprecated
• fixes some deprecated calls

Should I merge like that or do you plan to add other stuff in this PR ?

[carlocastoldi]

actually, when was the last time that the script with all the repeated defs worked? In the most recent version it gives a syntax error.

Do you still want to put a non-compilable script just so that people have to read and copy the parts that are interesting to them?
I feel like this shouldn't be the work of a script shipped with the extension. For that the website/documentation is perhaps the better place?

ERROR: startup failed:
QuPathScript: 65: The current scope already contains a variable of the name myObjectsWithinAList
 @ line 64, column 5.
   def myObjectsWithinAList = allRegions
       ^

QuPathScript: 81: The current scope already contains a variable of the name myObjectsWithinAList
 @ line 81, column 5.
   def myObjectsWithinAList = getAnnotationObjects()
       ^

QuPathScript: 85: The current scope already contains a variable of the name objectsOtherThan
 @ line 85, column 5.
   def objectsOtherThan = getAnnotationObjects() - myObjectsWithinAList
       ^

3 errors

[carlocastoldi]

is there a particular reason to why loadWarpedAtlasAnnotations() does not return the Root annotation?

Not really, I think it could return the root annotation without any problem.

I did it, in the end

should we fix the tests so that they're actually useful?

Sure, if you know how to do that, that'd be nice.

Perhaps this can be done in a future PR. I feel like it's something more important for the long-term support of this extension, rather than for immediate need

do you think we should make LoadAtlasRoisToQuPathCommand be a little be more complex?
But more importantly, what do you have in mind in terms of complexity?

I'd say the ability to display and choose between available atlas alignments

[NicoKiaru]

Oups, missed your replies, you were too fast...

I feel like this shouldn't be the work of a script shipped with the extension. For that the website/documentation is perhaps the better place?

Yes that's right. We can remove the script. We could add a command that would open the documentation instead. And the documentation needs to be rewritten.

I'd say the ability to display and choose between available atlas alignments

Wait, you have several alignments for the same image ?

[carlocastoldi]

Wait, you have several alignments for the same image ?

Not personally, no. But I've helped other laboratories that work on non-adult mice. Since their mice don't have an atlas perfectly designed on their age, they have to choose the lesser evil between the younger and the older one.

For this, they wanted to import both and compare the atlases on the single images, side by side

[carlocastoldi]

I beg your pardon for askin, but is there any reason this wasn't merged yet? :o

[NicoKiaru]

No reason, I asked above for your formal 'OK' and if you wanted to add additional stuff but this got lost with the other posts. But I suppose it's all ok on your side and there's nothing to add, so let's go for it.

[carlocastoldi]

Thank you!