Redundant deregistering of custom functions?

[gtfierro]

I recently filed an issue on RDFlib (RDFLib/rdflib#1492) documenting an exception I'm seeing when running SHACL validation with advanced and JS features turned on. I've spent some more time looking at the issue and I believe that --- while the idempotency of deregistering functions should probably be addressed --- there is a related issue in pySHACL.

When functions are being registered here, if a function is identified as more than one type (in my case, the dash:toString function is both a SPARQLFunction and a JSFunction), then it is added to the internal list of functions more than once, which leads pySHACL to attempt to remove it twice. However, the internal function lookup table is a dictionary so only one reference to the function can exist --- thus, removing it twice results in the exception I am seeing in the RDFlib issue.

My proposal is to change all_fns to a set rather than a list. How do you feel about that change? This should make sure that adding functions is idempotent and avoid the issue in RDFlib. I can submit a PR for that soon

[ashleysommer]

This was fixed in both PySHACL (#109) and in RDFLib (I think). So this issue can be closed now.