-
Notifications
You must be signed in to change notification settings - Fork 4
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
filtering/reconnecting #18
Conversation
js/main.js
Outdated
@@ -50,7 +50,7 @@ const mouseUp = function (event) { | |||
isDragging = false; | |||
|
|||
// console.time("drawAll"); | |||
drawAll(); | |||
drawAll(parentLinks, childrenLinks, infoBoxes); |
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.
Stupid question from my side, where do these inputs come from?
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.
Don't worry, these inputs come from main.js.
Hey @kjvbrt and @tmadlener, could you check my filtering function? As previously discussed, an external function determines whether to filter or not a particle. And then reconnected by using the |
I am rebasing this to potentially pick up the preview functionality. |
js/menu/filter.js
Outdated
} | ||
} | ||
|
||
function reconnect(criteriaFunction, parentLinks, childrenLinks, particles) { |
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.
I am not entirely sure whether this is necessary yet, but just for the future; Can this be written in a way where there are no parentLinks
? We will probably need that e.g. for the ReconstructedParticle
and I am wondering whether we should already spend the time now to prepare that?
Hey @tmadlener @kjvbrt, I started working on more advanced filtering for |
Very nice. Since you have the panel now already. Can I ask for one specific filter function which will should be straight forward to implement (from a technical point of view), but will probably require a bit of "leg work" from your side. The And then there is also some convenience functionality to check wheter a specifc bit is set: Would it be possible to have a filter that allows us to toggle these to filter out MCParticles for which specific bits are set? I.e. Have one checkbox / toggle for each bit and combine them into a filter function? IIUC all the filter combiners, etc. are already in place and this would mainly boil down to writing a specific filter for checking the bits in the |
Rebasing this again to see if the preview fixes from #22 are picked up. |
|
Preview is working 🚀 However, for me it doesn't show the filter panel. For testing the filtering, it would be nice to have some unittests for that to see if things work as expected. In these unittests you can just create some dummy MCParticle hierarchy with the values that you want in the end. You could start from the existing json file and just use a few of them and just adapt them to enable testing. This should all be possible without ever having to go to the UI to display them (for testing). |
Yeah, it's strange. I didn't expect this. I'll fix it.
Sure! In that case, I will wait for this PR and #20 to be merged, to add testing, because the testing setup is in #20 and this #18 has the function to be tested. So I think that after merging both PRs to main, I should create a separate PR to test filtering, and other PR to fix again PR previews, what do you think? |
I am quite happy with this. I think we can merge this, unless @kjvbrt has more comments. |
js/menu/filter/reconnect.js
Outdated
@@ -0,0 +1,70 @@ | |||
import { Link } from "../../objects"; |
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.
import { Link } from "../../objects";
->
import { Link } from "../../objects.js";
Otherwise it runs nicely. For next PRs we can think of showing only sim/gen status numbers which are used in the file. |
I run it again and found that the filter is reset when I move any of the elements. |
Sorry, I thought it wouldn't matter because it's just like a "prototype". But I'm going to make it work when moving too now. |
BEGINRELEASENOTES
ENDRELEASENOTES