Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

let paths be comparable against all nodes. Fixes #545 #552

Merged
merged 2 commits into from
Nov 29, 2015
Merged

Conversation

gromgull
Copy link
Member

No description provided.

@gromgull
Copy link
Member Author

Hard to say what happens with master failing ... i'll wait for it to be fixed and rebase :)

@joernhees
Copy link
Member

👍 sorry for that... #550

@joernhees joernhees added bug Something isn't working enhancement New feature or request in-resolution SPARQL labels Nov 24, 2015
@joernhees joernhees added this to the rdflib 4.2.2 milestone Nov 24, 2015
@joernhees
Copy link
Member

py3.4:

TypeError: unorderable types: URIRef() < AlternativePath()

i guess this is a symmetry problem... if the first operand is not a Path it breaks... should we maybe move the whole comparison code to Node or Identifier and make Path a subclass?

also, shouldn't https://github.com/RDFLib/rdflib/blob/master/rdflib/term.py#L171 actually raise NotImplemented() instead of returning it?

@gromgull
Copy link
Member Author

Don't ask me why, but returning is correct: https://docs.python.org/2/library/constants.html#NotImplemented

I'll look into py3, but not today :)

@joernhees
Copy link
Member

ah i didn't know it's actually a keyword and confused it with https://docs.python.org/2/library/exceptions.html#exceptions.NotImplementedError which would need to be raised...

@gromgull
Copy link
Member Author

The missing part was adding @functools.total_ordering - that fixes py3.4, but breaks 2.6 where it doesn't exist :(

When can we kill 2.6 support? :)

@joernhees
Copy link
Member

i guess with rdflib 5.0.0 but for now i just fixed it by also defining the other comps...

one other thing i noticed looking at this again though: if possible repr is meant to return something that you can paste into a python interpreter re-creating the object... do you see an easy way to change the __repr__ methods of the current Path objects in that way? (i didn't)

gromgull added a commit that referenced this pull request Nov 29, 2015
let paths be comparable against all nodes. Fixes #545
@gromgull gromgull merged commit 92aff84 into master Nov 29, 2015
@gromgull gromgull deleted the fix-issue-545 branch November 29, 2015 22:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working enhancement New feature or request in-resolution SPARQL
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants