sverchok
sverchok copied to clipboard
nortikin
Deal with case when inputs contains empty lists
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
- [x] Code changes complete.
- [x] Code documentation complete.
- [x] Manual testing done.
- [ ] Ready for merge.
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. What I think would be the simplest is to just have an error when the mask input is empty, and otherwise replace the empty list by a list of None objects the length ot the complementary list. This should solve my problem while absolutely not modyfying how it currently works.
The old version supported empty lists in the mask input too.
And I'm not sure about None values. As far as I know none of the nodes returns None. =)
Sverchok does not have tools to work with None values.
The old version supported empty lists in the mask input too.
I did not see this use case, so I can remove the error raising, no problem.
And I'm not sure about None values.
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.
This is how you currently can handle your problem. I don't see the point of injecting None values since they break the work of next nodes.
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. I used None default values because it will make the problem apparent, but I could have put a 0 default value, or anything else. Remember, the default value should not be chosen, if it is, it indicates that the node tree malfunctions, so raising an error right after the node is better than staying silent and raising one further down the tree.
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.
One level lists are not supported even by List Mask Join node.
It supports one level lists for Data True and False, but not Mask.
Nor rising an error nor inserting arbitrary values are not good solutions enough.
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.