Refactor Entry#to_rdf #83

Merged
merged 74 commits into from Jan 20, 2014

Conversation

Projects
None yet
3 participants
@tmaier
Contributor

tmaier commented Jan 17, 2014

This pull request is a work in progress and related to #82.
Critics, suggestions or hints for the conversion of more fields are highly welcome.

Todo

  • Add conversion for more fields
  • Add tests

tmaier added some commits Jan 17, 2014

Refactor Entry#to_rdf by introducing Entry::RDFConverter
Every conversion is in its own method. This allows proper teasing for every conversion and keeps its scope clear.

* Cleaned up BIBO_TYPES
* Added support for more fields
Update rdf gem dependency in Gemfile
There is no reason to limit ourself to this old version.
@inukshuk

This comment has been minimized.

Show comment
Hide comment
@inukshuk

inukshuk Jan 18, 2014

Owner

Looks great so far!

One minor suggestion: I would add an instance method #convert! or something like that that goes through all the methods instead of doing that in the constructor by default. I imagine this will give you more flexibility and make testing easier (you can create an instance and call individual converters or change the source object in between etc.).

Have you considered adding an RDF to BibTeX conversion as well? Would that be useful do you think?

Owner

inukshuk commented Jan 18, 2014

Looks great so far!

One minor suggestion: I would add an instance method #convert! or something like that that goes through all the methods instead of doing that in the constructor by default. I imagine this will give you more flexibility and make testing easier (you can create an instance and call individual converters or change the source object in between etc.).

Have you considered adding an RDF to BibTeX conversion as well? Would that be useful do you think?

@tmaier

This comment has been minimized.

Show comment
Hide comment
@tmaier

tmaier Jan 18, 2014

Contributor

The #convert! Thing is exactly something I thought about when I woke up today :)

Yes, I will definitely write a rdf to bibtex conversion. I just wonder if bibtex-ruby is the right place and where there exactly.

One more thing: I would like to avoid data loss when converting to rdf. Theoretically, it would be possible to mix in bibtexml into the rdf for fields which cannot be represented as expected. What do you think?

Contributor

tmaier commented Jan 18, 2014

The #convert! Thing is exactly something I thought about when I woke up today :)

Yes, I will definitely write a rdf to bibtex conversion. I just wonder if bibtex-ruby is the right place and where there exactly.

One more thing: I would like to avoid data loss when converting to rdf. Theoretically, it would be possible to mix in bibtexml into the rdf for fields which cannot be represented as expected. What do you think?

@inukshuk

This comment has been minimized.

Show comment
Hide comment
@inukshuk

inukshuk Jan 18, 2014

Owner

Unfortunately, I have too little practical experience using RDF to have an informed opinion on field mappings or mixing in BibTeXML. Perhaps you could ask @bdarcus for advice?

One more thing I noticed: please make sure to require the rdf gem only the first time that to_rdf is called — otherwise users who do not use RDF will be confused by the load error message.

Owner

inukshuk commented Jan 18, 2014

Unfortunately, I have too little practical experience using RDF to have an informed opinion on field mappings or mixing in BibTeXML. Perhaps you could ask @bdarcus for advice?

One more thing I noticed: please make sure to require the rdf gem only the first time that to_rdf is called — otherwise users who do not use RDF will be confused by the load error message.

@bdarcus

This comment has been minimized.

Show comment
Hide comment
@bdarcus

bdarcus Jan 18, 2014

RDF is an general data model. BibTeX is a very specific one. So it doesn't make sense to talk aboutconverting from RDF, unless you talk specific vocabularies. I'd leave it out.

bdarcus commented Jan 18, 2014

RDF is an general data model. BibTeX is a very specific one. So it doesn't make sense to talk aboutconverting from RDF, unless you talk specific vocabularies. I'd leave it out.

@tmaier

This comment has been minimized.

Show comment
Hide comment
@tmaier

tmaier Jan 18, 2014

Contributor

Yeah, this is the reason why I'm not sure if bibtex-ruby is the right place for it.

But coming back to adding bibtexml to the rdf. Is this ok (as an addition to bibo)?

Contributor

tmaier commented Jan 18, 2014

Yeah, this is the reason why I'm not sure if bibtex-ruby is the right place for it.

But coming back to adding bibtexml to the rdf. Is this ok (as an addition to bibo)?

@bdarcus

This comment has been minimized.

Show comment
Hide comment
@bdarcus

bdarcus Jan 18, 2014

RDF/XML allows you to embed XML within a property, so long as you use the
right attribute (per the RDF/XML spec).

But you need to think about the trade-offs in doing that. Yes, you preserve
the data, but if that RDF gets consumed more broadly, it may be that the
data can't get properly displayed.

On Sat, Jan 18, 2014 at 9:25 AM, Tobias L. Maier
notifications@github.comwrote:

Yeah, this is the reason why I'm not sure if bibtex-ruby is the right
place for it.

But coming back to adding bibtexml to the rdf. Is this ok (as an addition
to bibo)?


Reply to this email directly or view it on GitHubhttps://github.com/inukshuk/bibtex-ruby/pull/83#issuecomment-32683013
.

bdarcus commented Jan 18, 2014

RDF/XML allows you to embed XML within a property, so long as you use the
right attribute (per the RDF/XML spec).

But you need to think about the trade-offs in doing that. Yes, you preserve
the data, but if that RDF gets consumed more broadly, it may be that the
data can't get properly displayed.

On Sat, Jan 18, 2014 at 9:25 AM, Tobias L. Maier
notifications@github.comwrote:

Yeah, this is the reason why I'm not sure if bibtex-ruby is the right
place for it.

But coming back to adding bibtexml to the rdf. Is this ok (as an addition
to bibo)?


Reply to this email directly or view it on GitHubhttps://github.com/inukshuk/bibtex-ruby/pull/83#issuecomment-32683013
.

tmaier added some commits Jan 18, 2014

Add BibTeXML fallback
The fields are added directly to the entry.
BibTeXML would usually require an <bibtex:entry> tag followed by e.g. <bibtex:book>. <bibtex:book> would then contain the fields.
As <bibtex:entry> nor <bibtex:book> are proper predicates, I omitted them for now.
Add series
Will be just added if it does not have the same :title, :series or :issn as the parent
@tmaier

This comment has been minimized.

Show comment
Hide comment
@tmaier

tmaier Jan 19, 2014

Contributor

Almost done.

I didn't come up with a good way to test the graph. I would appreciate some ideas.

Contributor

tmaier commented Jan 19, 2014

Almost done.

I didn't come up with a good way to test the graph. I would appreciate some ideas.

@inukshuk

This comment has been minimized.

Show comment
Hide comment
@inukshuk

inukshuk Jan 19, 2014

Owner

Great job! Thanks also for moving the converters to separate files and tidying up.

What's your own use case for the RDF export? In my experience this usually gives you a good test case (e.g., some expectation you have on the graph object ) if it's possible to strip it down to the bare minimum. Any ideas?

Otherwise just let me know when you want this merged; I'll also push this out to rubygems afterwards.

Owner

inukshuk commented Jan 19, 2014

Great job! Thanks also for moving the converters to separate files and tidying up.

What's your own use case for the RDF export? In my experience this usually gives you a good test case (e.g., some expectation you have on the graph object ) if it's possible to strip it down to the bare minimum. Any ideas?

Otherwise just let me know when you want this merged; I'll also push this out to rubygems afterwards.

tmaier added some commits Jan 20, 2014

Add agent to initializer
key of #agent could be BibTeX::Name. And this would be always a different object. This is why we use then key#to_hash as the key.
Fix #parent and #children
As we did not check, if an Entry was already in our graph, the converter would run infinitely.
Fix Entry#contained?
Some applications, like BibDesk set the title and the booktitle to the same value. This would lead to a false positive when calling #contained?
Fix #year
Output was "2014-feb". Is now "2014-2"
Remove bibo:created and bibo:issued from #year
They do not exist. (Links to Dublin Core)
Use DC.issued only in #year
bibo says about DC.issued: "Used to describe the issue date of a bibliographic resource"
whereas it says about DC.created "Used to describe the creation date of a bibliographic item".
Bibliography#to_rdf
Every entry was included multiple times in the graph returned, as each conversion of an entry did not know of each other when it created the children and parent

The code style follows Entry#to_rdf
@tmaier

This comment has been minimized.

Show comment
Hide comment
@tmaier

tmaier Jan 20, 2014

Contributor

Alright. I consider it as done for now.

The only thing what should be highlighted in the README is, that one needs to convert the LaTeX code to UTF-8 before exporting to RDF or anything else.

convert(:latex).to_rdf

Please have an extra look at 171983f. This is a change which is not really related with this pull request. If you're unhappy with this, please let me know. I would remove it from here and open a new PR just for this change.

Contributor

tmaier commented Jan 20, 2014

Alright. I consider it as done for now.

The only thing what should be highlighted in the README is, that one needs to convert the LaTeX code to UTF-8 before exporting to RDF or anything else.

convert(:latex).to_rdf

Please have an extra look at 171983f. This is a change which is not really related with this pull request. If you're unhappy with this, please let me know. I would remove it from here and open a new PR just for this change.

inukshuk added a commit that referenced this pull request Jan 20, 2014

@inukshuk inukshuk merged commit 153c3cb into inukshuk:master Jan 20, 2014

1 check passed

default The Travis CI build passed
Details
@inukshuk

This comment has been minimized.

Show comment
Hide comment
@inukshuk

inukshuk Jan 20, 2014

Owner

That's fine. Landed in 3.1.0. Cheers!

Owner

inukshuk commented Jan 20, 2014

That's fine. Landed in 3.1.0. Cheers!

inukshuk added a commit that referenced this pull request Jan 20, 2014

Merge pull request #84 from tmaier/master
Follow-up to #83 - RDFConverter

@tmaier tmaier referenced this pull request Jan 22, 2014

Closed

RDF Export #51

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment