-
Notifications
You must be signed in to change notification settings - Fork 5
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
Registration of Two Lineages #10
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi,
I have only a couple of minor comments that I believe could improve the readability of the code, plus a few things that might be worth double-checking .. all remarks are given in the comments to the code.
The algorithm itself is I think reasonable given its expected setting, and good that it echos the expected settings in the dialog.
I have to honestly admit that I haven't paid too much attention to the tests... and haven't yet found time to compile and test it.
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageTreeUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageTreeUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageTreeUtils.java
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/RootsPairing.java
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationPlugin.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/EstimateTransformation.java
Outdated
Show resolved
Hide resolved
...main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationAlgorithm.java
Outdated
Show resolved
Hide resolved
...main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationAlgorithm.java
Outdated
Show resolved
Hide resolved
...main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationAlgorithm.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageTreeUtils.java
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationDialog.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationDialog.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationDialog.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationDialog.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationDialog.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/RootsPairing.java
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/RootsPairing.java
Outdated
Show resolved
Hide resolved
src/test/java/org/mastodon/mamut/tomancak/lineage_registration/Glasbey.java
Outdated
Show resolved
Hide resolved
src/test/java/org/mastodon/mamut/tomancak/lineage_registration/Glasbey.java
Outdated
Show resolved
Hide resolved
.../java/org/mastodon/mamut/tomancak/lineage_registration/LineageRegistrationAlgorithmTest.java
Outdated
Show resolved
Hide resolved
* An transformation between the two dataset is estimated from the labeled spots in the first time point of the two datasets * The tree is sorted such that the spindle direction matches between the two datasets. Scalar product > 0.
The EstimateTransform method uses code that was probably implemented as part of the multi-view reconstruction plugin for Fiji. But we furtunately don't need the full multi-view reconstruction dependency.
This is a preparation step. There is a significant difference between the LineageRegistrationPlugin and other plugins in this repo. The lineage registration plugin will operate on multiple open Mastodon projects. It therefor needs some specialized MastodonPlugin.
The updated dialog supports a wider range of operations.
a040706
to
14d654b
Compare
13121eb
to
7fb432d
Compare
(fixed) It would be good, if the Lineage Registration Dialog would always come to front, when "Plugins" > "Lineage Registration" is selected. Otherwise, the user may have forgotten that the dialog is already open and just not visible anymore, because other Windows are in front of it. Consider e.g. the following workflow: |
The lineage registration dialog shows Mastodon projects that are not open anymore: Before this screenshot, I did the following:
Result: four project are shown in selection dialog, while only two projects are open. The function "Update list of projects" does not seem to change anything in list of available projects, even though the name of that function would indicate that. Also the lists of tag sets in the "copy tag set function" does not seem to be correctly updated (shows tag sets of projects that are not open anymore). |
Would it be possible to automatically, regenerate the branch graph after sorting the TrackScheme via the Lineage Registration dialog. At the moment, if you are not an experienced user, it may be easily overseen that the icon to regenerate the branch graph in the sorted project changed and that only after pressing this button the actual sorting visually happens. |
Microsoft Windows treats JDialog and JFrame differently. A JDialog does not show up in the task manager. Extending JFrame therefore improves usability. There are also other reasons for this change. The LineageRegistrationDialog does not behave like a dialog: It is not modal. There are no Ok or Cancel buttons. There's only a close button. It's meant to stay open for a longer duration, while interacting with other windows. It therefor behaves more like a JFrame.
Well spotted. I'm aware of this problem. But I don't know no way for this plugin to get informed when the Mastodon project is closed. The plugin API has no unregister method MastodonPlugin. I tried to use WeakReferences but this didn't work. Mastodon projects are never garbage collected (on linux). So there is also garbage collection bug in Mastodon.
The button adds newly opened projects to the list. It doesn't remove the already existing entries. I would like to get rid of this button but it's required for stupid reasons. When initializing a plugin the project path is not yet set, so it's not possible to update this list, as the project name cannot yet be deduced. This means I cannot reliably update this list when a new mastodon project is opened. This button is a workaround for that problem.
I cannot reproduce this problem. The list behave correctly as far as I can tell. Please provide a more detailed problem description. |
There are only hacky ways in java swing to show a tooltip longer. Please keep in mind that there are currently only two intended users for this plugin. Too much time invested here is time wasted! |
I also had this idea. But no, we shouldn't do this. I learned from some provided datasets that projects name tend to become very long. So having only the short letters is best for readability. |
This should work. The projects should get a tag set called "lineage registration" added. Could you verify if this problem persists. Please tell me how to reproduce the problem. For me the functionality works as expected. |
@stefanhahmann & @xulman when reviewing this, please keep in mind that I currently only expect two users for this plugin: Johannes and Mette. Tell me if you see significant problems with the functionality or code. There is no need to polish all the little details. I want to make this algorithm available for now. Let's see if the algorithm works at all, it probably still need a lot off improvement and the UI will need to change with it. This plugin needs to be considered a prototype! |
This drastically improves the feedback to the user if the sorting the TrackScheme is used together with the "TrackScheme Branch" or "TrackScheme Hierarchy" window. As the user no longer needs to klick. The tiny "regen." button.
I agree. Hadn't thought of long filenames.
I agree. Thanks for clarification.
I did not know this. Thanks for making that clear.
That was unclear to me. Thanks for making that clear.
It works as intended. It did not understand that expected result are 2 tag sets, but this is actually said in the tooltip. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All the acceptance criteria seem to work now. This is very good.
Thanks also for the UI improvements. Especially that the Dialog is now a JFrame is an usability improvement.
There are some minor comments leave. Can you please have a look on them?
The coupling mechanism may deserve a more detailed code review. I did only a coarse one for the sake of the getting this to the user.
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageTreeUtils.java
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/RootsPairing.java
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/LineageTreeUtils.java
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/RefMapUtils.java
Outdated
Show resolved
Hide resolved
src/main/java/org/mastodon/mamut/tomancak/lineage_registration/RootsPairing.java
Show resolved
Hide resolved
Previously those cells that had differently sorted child cells where tagged. This commit changes the code such that the child cells themselves get tagged. Which is hopefully more intuitively to understand. Incoming edges are tagged too. The order of the buttons in the LineageRegistrationFrame is changed. Tagging cells is usually the first action a user would want the use. The buttons for tagging cells are therefor now in the top row.
Add a plugin that sorts the TrackScheme of one Mastodon project to match the order in another project.
Sorting a TrackScheme here is understood as sorting the order of child nodes for every dividing spot in the TrackScheme.
The algorithm implemented here does this by calculating the division directions for each cell division in both embryos. The division direction are compared between the two embryos in order to sort the child nodes of every dividing spot in the same way.
Requirement for the algorithm:
Steps of the algorithm:
TODOs
Acceptance criteria