there is no use-case of comparing a Literal with None (I think)

[FlorianLudwig]

This is somewhat of a question: What is the use-case of comparing a Literate instance to None? I didn't find any, so I removed the checks from the comparing functions and all tests still passed.

[coveralls]

Coverage decreased (-0.1%) to 75.487% when pulling 99a8a0f on FlorianLudwig:comparing_literal_with_none into 652d2e6 on RDFLib:master.

[white-gecko]

In my eyes this is ok. The instance check will return False for None and thus we will end at NotImplemented or False in the case of equals, which is good. As we are heading towards a major release breaking changes are ok.

[nicholascar]

@ashleysommer and I talked about this sort of thing at our last meeting a couple of weeks back. Ashley, what did we decide here?

[ashleysommer]

We discussed the merits of storing None vs False when parsing an invalid xsd:boolean literal. Technically both are wrong, but we need to choose one or the other (or throw an exception), and all literals must follow consistent rules.

However, that is not related to this Issue/PR.

The only thing I can see this is a maybe problem here is the possibility of breaking Literal Total Ordering. Given that feature is only concerned with comparing Literals with Literals, this change is probably fine.

As @white-gecko said, this is a breaking change. Users with code which expects Literal(1) > None to evaluate to True will now get a NotImplemented object/type returned. That will be a weird bug to hunt down for those poor people. Would it be better to actually throw a NotImplementedError() instead?

[FlorianLudwig]

I would expect a TypeError raised, to be honest. NotImplemented is returned as it is checked within the sparql evaluation code to throw a SPARQLError, See:

https://github.com/RDFLib/rdflib/blob/master/rdflib/plugins/sparql/operators.py#L857

Interestingly raising a TypeError would actually lead to the same result.

[FlorianLudwig]

On a somewhat related note, I have yet to understand when SPAQRL is supposed to throw an error and when silently just return an empty literal. For example DAY(xsd:Date) results in an empty string and I would have expected a TypeError as DAY() only accepts xsd:DateTime.

Pointers on the expected behaviour are welcome as I would like to do some refactoring on the sparql evaluation :) [with this MR being the first step]

[white-gecko]

In case of DAY() the spec states

This function corresponds to fn:day-from-dateTime.

In the "XPath and XQuery Functions and Operators 3.1" Recommendation there is a statement about errors: https://www.w3.org/TR/xpath-functions/#errors-and-diagnostics also some functions have Error Conditions specified. I assume if no Error Conditions are specified, the function returns no errors.

Also for the operators I think to be sure it is necessary to dive into the XPath specs: https://www.w3.org/TR/sparql11-query/#OperatorMapping

[ashleysommer]

Can we change this to throw TypeError() when trying to compare Literal to None? Seems like that is the correct solution.

[ashleysommer]

Whats going on with the indentation here? This line is offset by two spaces?