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

naming ambiguity rdflib.URIRef / rdflib.Literal etc (and non lower case module names) #41

Closed
ghost opened this issue Feb 20, 2012 · 0 comments

Comments

@ghost
Copy link

ghost commented Feb 20, 2012

eik...@gmail.com, 2009-02-14T21:28:16.000Z

The "from rdflib.URIRef import URIRef" in rdflib's __init__.py clobbers rdflib.URIRef as the name of 
the module. This is confusing. Also, nose trips up on this naming ambiguity between the module 
and class; and as a result can not run the test cases.

Comment 1 by eik...@gmail.com
Committed changes in r1483 to fix all of the naming clashes except for the three InputSource ones. Just need to 
figure out where we want to put those three classes rdflib.syntax.source ?


Comment 2 by gsf747@gmail.com
What about naming ambiguities between classes and the modules in rdflib.store?  An
example would be "from rdflib.store.Sleepycat import Sleepycat".  They're not
clobbered in __init__.py or causing test errors now, but they have the potential.


Comment 3 by manu.a...@gmail.com
Hi,

I want use the rdflib.Literal('something', language=None,
datatype=rdflib.URIRef('http://www.w3.org/2001/XMLSchema#string')) as a key value for
dictionary Literal_values.

But I have got an error

How can I use it as key value for a dictionary in python?

Any help would be appreciated

Best,
Mnau


Comment 4 by drewpca
manu.alem, your issue seems unrelated to this bug regarding rdflib imports. If you're 
still having a problem, please post it as a new issue and remember to include your 
exact error message so we can fix it.


Comment 5 by drewpca
Regarding the imports, work has already been done to put Literal/etc in a term.py 
module. But meanwhile, imports that have worked for years are now broken. I shall 
prepare a patch that makes "from rdflib import Literal, URIRef, BNode, Variable" work 
again. I'll also see what it would take to make things like "from rdflib.Graph import 
ConjunctiveGraph" work but print a deprecation warning. If legacy projects can't even 
start up with rdflib 2.5, they're going to hate moving to it.


Comment 6 by lindstr...@gmail.com
While still supporting rdflib.Graph et.al. is very nice for 2.4.x-users (whose code will break otherwise), 
IMHO this breaking change is for the better at large. I have faith that this wouldn't hurt the people 
using 2.4.x too much, as long as we're explicit and helpful in informing about this (and why it's been 
done).

As for supporting convenience import, I think it's both good for those who have used it in 2.4 (I did), 
and for the future as well. I have this for "rdflib/__init__.py"::

    # import common classes, functions and constants:
    # IMPORTANT: make sure to *never* shadow module names with these imports!
    from rdflib.graph import (Graph, ConjunctiveGraph,
            QuotedGraph, ReadOnlyGraphAggregate,
            ModificationException, UnSupportedAggregateOperation)
    from rdflib.namespace import Namespace, RDF, RDFS, OWL, _XSD_NS as XSD
    from rdflib.term import BNode, Identifier, Literal, URIRef, Variable

The big difference from 2.4.x is (at least) that Graph is exposed directly, instead of rdflib.Graph being 
BackwardCompatGraph. I think it's fairly ok now not to expose that though (it's been deprecated for 
quite a while AFAIK).


Comment 7 by lindstr...@gmail.com
Still, do we need an explicit declaration about exactly which namespace changes are ok to go into 
2.5? If we want to do more, should we defer all the rest to 3.0, or progressively rename (e.g. 
lowercase, relocate) "internal" packages for 2.6 etc.?

I'm unsure about how many 3d-party packages import from the internals of syntax etc.. (And what 
tolerance those would have regarding any changes.)


Comment 8 by gromgull
I disagree with lindstream, and semi agree with drewpca. 

I think being able to do: 

import rdflib

and then rdflib.Graph, Literal, etc. 

is a great usability feature. It's annoying to write the extra module in the middle
of rdflib.graph.Graph, we should really try to make working with rdflib as easy as
possible.


Comment 9 by lindstr...@gmail.com
Sorry, I was unclear. We really do agree. :) I just meant that I don't think we need to support the 2.4.x 
uppercase-modules (as I interpreted it drewpca was looking into).

So, no to the old (and currently fixed, i.e. removed):

    from rdflib.Graph import ConjunctiveGraph 

The new full forms are of course (e.g.):

    from rdflib.graph import ConjunctiveGraph
    from rdflib.namespace import Namespace
    from rdflib.term import URIRef

But *also* support:

    from rdflib import ConjunctiveGraph, Namespace, URIRef

I.e. add back the convenience imports in "rdflib/__init__.py".


Comment 10 by eik...@gmail.com
I'm going to work on adding the short forms back in now. I do not plan on adding back the upper case modules 
that were clashing with the class names however.


Comment 11 by eik...@gmail.com
This issue was updated by revision r1768.

moved


Comment 12 by eik...@gmail.com
This issue was updated by revision r1769.

Forgot to add this as part of r1767


Comment 13 by drewpca
The revisions in the last two comments seem to be about some other thing.

Anyway, what is the decision for the __repr__ of URIRef, Literal, etc? Since 
rdflib.URIRef will work, I would like the repr to be shorter:

  GOOD rdflib.URIRef('http://example.com/')
   BAD rdflib.term.URIRef('http://example.com/')

I have implemented the shorter repr and refactored the repetitive __repr__ code in 
branches/repr-43. Note that URIRefs and BNodes now emit unicode strings, which is a 
small change. I think that's correct given that URIRef and BNode are subclasses of the 
unicode type.

The tests are passing in that branch, feel free to just merge it in if you approve of 
the new reprs.

http://code.google.com/p/rdflib/source/detail?r=1793 has the diffs


Comment 14 by eik...@gmail.com
Sure; let's go ahead and treat the shorter names as the canonical name. The __repr__ methods, however, are 
supposed to return string objects; see: http://docs.python.org/reference/datamodel.html#object.__repr__

As for the last couple checkins, I've also been lumping in modules with non lower case names into this issue. I've 
modified the summary accordingly.


Comment 15 by eik...@gmail.com
This issue was closed by revision r1827.


Comment 16 by eik...@gmail.com
This issue was updated by revision 0503aa1c8716.


Comment 17 by eik...@gmail.com
This issue was updated by revision fde85e9b6c41.

moved


Comment 18 by eik...@gmail.com
This issue was updated by revision ed78f9ad9a6c.

Forgot to add this as part of r1767


Comment 19 by eik...@gmail.com
This issue was updated by revision 59e4a5955a32.


Comment 20 by eik...@gmail.com
This issue was updated by revision a42eecceb5db.


Comment 21 by eik...@gmail.com
This issue was updated by revision db54fad4b5b8.


Comment 22 by eik...@gmail.com
This issue was updated by revision f63201454234.


Comment 23 by eik...@gmail.com
This issue was updated by revision ee41728ab2e1.


Comment 24 by eik...@gmail.com
This issue was updated by revision b643c3c0c7e3.


Comment 25 by eik...@gmail.com
This issue was updated by revision deb907127c67.
@ghost ghost closed this as completed Feb 20, 2012
This issue was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

0 participants