Deal with case when inputs contains empty lists #4664
Conversation
|
What do you think about case when every list is empty? (Length of mask/data_t/data_f == 0). Your code rise exception but I think this case is not an error. |
|
It seems works perfectly normal with empty lists already.
Raising an error just makes the work more inconvenient. It's normal situation when some objects are empty lists and nodes should be able to handle them without blocking execution of next nodes. |
|
No it does not work fine, because whatever you do, you will never be able to get data from your list [1, 2, 3], but you definitely should be able to. So, instead of raising an error, maybe only picking the data which exists, and ignoring the rest is the way to go? However, if the mask is empty, it should still raise an error, since the node cannot do anything without a mask, right? |
|
The principles are next - any object can be an empty list, empty ojects should not prevent a node from handling nonempty objects. And I guess if an input is mandatory and it is an empty list then output also should be an empty list. I don't think we should make nodes too much clever. Users always can replace empty lists with something else if they need. |
|
Currently, an empty mask does prevent the node from doing anything, so it should raise an error. Empty true or false data do also prevent dealing with this specific pair of inputs, and as I explained, sometimes the whole point of the node is to deal with empty lists on one of the inputs. I will perform the modifications so that the node works even in this case, and precise on the documentation how this case is handled. |
|
Is my pull request okay for merge, now, @Durman? |
|
The old version supported empty lists in the mask input too.
And I'm not sure about
Sverchok does not have tools to work with None values. |
I did not see this use case, so I can remove the error raising, no problem.
What would you consider as default value instead of None? Maybe having None values crash following nodes could help detect that the mask used is wrong as it tries to choose data that does not exist. |
|
Empty lists are not something wrong. They should not break nodes work. I'm, actually, quite satisfied with how the node currently works without the proposed changes. |
|
You may be, but I am not, since it makes my node tree unusable in some cases, and there is no other way to circumvent the problem. Empty lists are not wrong, what is wrong is that when one list is empty, the output is automatically empty, even if the data of interest is in a non-empty list. So, non-empty data should be retrievable regardless of the other input list, and the only way, with the matching length mechanism, is to fill empty list with default data, and then, to either choose data as normal, or to ignore chosen default data, but I cannot imagine anyone would want the latter, and if it ever happened, there would be the risk of a silent problem arising. |
|
|
|
That is not my exact problem case : both my Data True and Data False are one level lists : [a, b, c, ...], and sometimes one of them is empty : []. In your example, the Switch node works by default on second level, so I would have to wrap and then unwrap my data. Moreover, the Switch seems to have a length matching function, so the same problem as with List Mask Join will arise. |
|
One level lists are not supported even by List Mask Join node.
Nor rising an error nor inserting arbitrary values are not good solutions enough. If a node does not have enough input data the output is empty list. It's quite clear and simple rule. Probably you can rather make a proposal of modifying Filter Empty Objects to make it work to solve your problem or make a proposal for new node which would replace empty lists with something else. |
It supports one level lists for Data True and False, but not Mask.
There is a third solution : ignoring values that do not exist, which would mean that if Data True is [1, 2, 3] and Data False is [], and Mask is [1, 0, 1], the output would be [1, 3] instead of [1, default, 3]. Would that be good enough? |
|
Such solution is better because it does not break next nodes. But, at least in some cases, it can mix things up.
In the example there is an object which polygons have random colors. The task is to replace random colors with black one for some polygons and it's possible with List Mask Join node. But, for example, instead of black color there is an empty list for some reason. Now the output of List Mask Join node also will be an empty list and it will make all polygons to be black. With your proposal the output list won't be empty, order of values will be broken and polygons will lost their initial colors. So at least in this case your proposal does not have sense. |
|
Okay, new idea : instead of adding an arbitrary default value, add an input which value is used as default value. If no value is given it defaults to None or output empty lists, as currently. No need to create a new node, it would still be backward compatible. |
|
In this case the node have to get two new inputs I guess - default values for True and default values for False. I think this is not so frequent problem to expend the node interface like that. A separate node can handle this much better. |
|
Not frequent? It seems something really frequent to me, as it is there to deal with the case when you want to iterate with for each node on a list that is sometimes empty. If you know another way to deal with this easily, I'm all ears, as it is a problem I encounter in most of my trees. Creating a new node seems too much redundant, just for dealing with a corner case like this. |
|
I'm judging on number of reports about problems with empty objects. You probably can create an issue to describe your problem so it would be clear what is the motivation behind the PR and there might appear another solution. |
Addressed problem description
Sometimes, the List Mask Join (In) node can be used to switch between empty lists and non empty ones, in order to make sure no empty list get to the rest of the treatment. In this case, the chosen data with the mask is of course that of the non-empty lists, but the node outputed nothing because of the match length option.
Solution description
So, I changed how matching length was done so that it works fine even in this case, while preserving how the node works in other cases. While I was at it, I added ValueErrors when mask is empty, both true and false data are empty, or data from an empty list is selected with the mask.
Preflight checklist