knime-ip / knip-scijava

KNIP - SciJava Commands Plugin
2 stars 3 forks source link

Missing Cell Handling #45

Open dietzc opened 8 years ago

dietzc commented 8 years ago

If MissingValue cells are mapped into a module, we should handle this gracefully. If null is returned by the module, we should return a MissingCell.

Squareys commented 8 years ago

Theoretically, this is handled by the Converter Framework. That depends on how they are cached. If you require a converter from MissingValue -> (Any) Object, you automatically get a converter which always returns null.

I wonder how that works for the other way around... Since you cannot do nulledObject.class and null instanceof Object == false.

Squareys commented 8 years ago

I added a test: MissingValue handling seems to work perfectly fine in #49.

dietzc commented 8 years ago

What happens for incoming MissingValues? Actually, the node should fail gracefully with a nice error message if a required=true input is null. For required=false the node shouldn't fail.

If it produces MissingValues as output, we have to decide what we do: Create missing-cells for all inputs or let the node fail?. I prefer fail with the corresponding error-message from the Command in the console log because it's more transparent to the user (node isn't green and has tons of MissingValues.). @hornm any comments?

Squareys commented 8 years ago

Actually, the node should fail gracefully with a nice error message if a required=true input is null. For required=false the node shouldn't fail.

That's an important note. This basically means that MissingValues should not be allowed to even get to the Converter Framework, but handled beforehand?

If it produces MissingValues as output, we have to decide what we do: Create missing-cells for all ~inputs~ (outputs?) or let the node fail?

Why for all outputs? Some outputs may still be valid and useful? We can still put an error message into the missing cell and possibly produce a Warning.

dietzc commented 8 years ago

That's an important note. This basically means that MissingValues should not be allowed to even get to the Converter Framework, but handled beforehand?

for required=true MissingValues shouldn't be allowed.

Why for all outputs? Some outputs may still be valid and useful? We can still put an error message into the missing cell and possibly produce a Warning.

Sounds good, however I'd avoid to produce warnings for each row. Rather I'd suggest that we have a warning summary after the node has finished (like K-Errors and N-Warnings occured, see log for details). And in the log we have more detailed messages on e.g. log-debug level.

Squareys commented 8 years ago

I'd avoid to produce warnings for each row

How about: "K-Errors and N-Warnings occured, see missing cells for details" :P

dietzc commented 8 years ago

I think we are a bit off-topic now: So for MissingCell input/output the strategy is clear I guess. The Exception handling will be discussed in #53.