search

scijava / scijava

Foundational libraries for scientific Java applications 🧱
https://scijava.org
6 stars 0 forks source link

Migrate all mesh-related Ops from scijava-ops-image to imglib2-mesh #232

Open ctrueden opened 3 weeks ago

ctrueden commented 3 weeks ago

It would be best for the imglib2-mesh library to have static methods for doing all the mesh operations and statistics currently provided by scijava-ops-image e.g. in the geom3d subpackage. And they can be exposed as Ops by using @implNote op in the imglib2-mesh codebase. No need to keep them all locked up inside scijava-ops-image.

gselzer commented 2 weeks ago

@ctrueden how would you like to handle the tests? It would be nice to test that all declared Ops are actually available like we think they are...

ctrueden commented 2 weeks ago

It would be nice to test that all declared Ops are actually available like we think they are...

This is a more general question than only the mesh/geom3d Ops, no? I.e. you are asking what our policy is on testing Op matching versus business logic behavior... right?

It looks like scijava-ops-image currently has 433 distinct Op names:

grp -oh "@implNote op name\?s='[^']*'" src/main | sort | uniq -c | sort -nr | wc -l
433

only 244 of which are currently being tested to match:

grp -oh '\.op([^)]*)' src/test | grep -v '"test\.' | sort | uniq -c | sort -nr | wc -l
244

My feeling until now is that it is not critical for us to test that Ops are actually present in the environment and actually match. I recognize that that feels bad, but it would be a pretty boilerplate test, and there is a question of where it ends, w.r.t. all the various adaptations and conversions that can occur in matches.

If you disagree and would like to test that every declared Op successfully matches a down-the-middle request, I support that, as long as we introduce such matching tests as uniformly as possible across the entire suite of Opified components (scijava-ops-image, scijava-ops-flim, imglib2-mesh, imglib2-algorithm, etc.). Do you have thoughts on how you want to approach it? I would also prefer that we test matching only, rather than matching+execution.

gselzer commented 2 weeks ago

If you disagree and would like to test that every declared Op successfully matches a down-the-middle request, I support that, as long as we introduce such matching tests as uniformly as possible across the entire suite of Opified components (scijava-ops-image, scijava-ops-flim, imglib2-mesh, imglib2-algorithm, etc.). Do you have thoughts on how you want to approach it? I would also prefer that we test matching only, rather than matching+execution.

Well the main thing that I want is to ensure that every @implNote turns into an OpInfo that could be matched - I don't think it's terribly important that we actually test the matching, because then there are other questions to answer e.g. priority. I'd think that could be automated easier...

ctrueden commented 2 weeks ago

Is something like the OpRegressionTest good enough? Where there is just one test that queries the environment for all available OpInfos, and checks that they match an expected list?

gselzer commented 1 week ago

Is something like the OpRegressionTest good enough? Where there is just one test that queries the environment for all available OpInfos, and checks that they match an expected list?

I mean, yes, however we'd still have to figure out where that test lives. I think it should not live within imglib2-mesh, but then I have no idea where it would live.