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

trig output fixes #480

Merged
merged 6 commits into from
Apr 1, 2015
Merged

trig output fixes #480

merged 6 commits into from
Apr 1, 2015

Conversation

drewp
Copy link
Contributor

@drewp drewp commented Mar 30, 2015

No description provided.

Before: "<short:name> {...}" and "<None> {...}"
After: "short:name {...}" and "<http://actual/uri/here> {...}"
@coveralls
Copy link

Coverage Status

Coverage increased (+1.22%) to 63.33% when pulling bd57db7 on drewp:master into 395a401 on RDFLib:master.

p = triple[1]
if isinstance(p, BNode):
self._references[p]+=1

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

did you remove this on purpose? seems it is called from preprocess and the one in TurtleSerializer looks quite different to me

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TurtleSerializer's one is a super call followed by a repeat of lines 48-57. The super call would run RecursiveSerializer's method, which is a repeat of lines 45-47. Most importantly, the test suite still passes :)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah sorry, i must have looked at the RecursiveSerializer one directly... so essentially this was unnecessary code duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I think so

@joernhees joernhees added this to the rdflib 4.2.1 milestone Apr 1, 2015
@joernhees joernhees added bug Something isn't working serialization Related to serialization. labels Apr 1, 2015
@joernhees
Copy link
Member

thanks for the changes and comments, i think this looks good now...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working serialization Related to serialization.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants