Open MaxOstrowski opened 1 year ago
It would be easy to change but I'm not sure if it is a good idea.
The unification process seems to be a common place for things to go wrong. For example, cases where the user specifies the wrong field in their predicate definition so the facts that they expect to be unified aren't. In general I tend to feel that it makes clorm easier to use if it can detect and report as many mistakes as possible (... and of course, improving the error messages would also help :)) .
With this in mind, I can't think of a case where passing None
is not a mistake. However, I'm less certain about the empty list. In most use cases that I can think of it seems to be an obvious error and it would help the user of the library if the function threw an exception. But maybe in your case it might sense. If so maybe we could add a flag to control if an exception is thrown or not?
In your case, if the list of unifiers is given externally does it actually make sense for a user of your system to pass an empty list? Wouldn't it also be a red-flag that the user has done something wrong (in which case catching this exception would help you to report back to the user)?
I agree and I do not have a strong opinion about it. In my case I just needed some default value to start coding with and see if everything works. This is not the best use case. There is no actual user interaction, this was a fictional scenario. I could imagine that sometimes you might be interested in some atoms of the solutions and sometimes only in SAT or the optimality value. In this way this could be done with the same code without any 'if'. If this is a good idea in terms of error detection I actually don't know. Maybe someone else has an oppinion about it or there are similar functions in other ORM libraries ... Sorry
Calling
unify
with an empty Predicate list or None causes the following error:ValueError: The unifier must be a list of predicates or a SymbolPredicateUnifier
Would it make sense to allow the empty list (or None) and return an emptyFactBase
? In my case, the list is given from outside (maybe user input, maybe problem dependent, and this would make it more general, like a default case. Do you think this makes sense ?