ruby-rdf / sparql-client

SPARQL client for Ruby.
http://rubygems.org/gems/sparql-client
The Unlicense
112 stars 58 forks source link

malformed prefix example in docs #89

Closed ConorSheehan1 closed 5 years ago

ConorSheehan1 commented 5 years ago

I noticed that the example in the docs for query.prefix produces a malformed query

require 'sparql/client'

client = SPARQL::Client.new('http://dbpedia.org/sparql')
query = client.select.
  prefix(dc: RDF::URI("http://purl.org/dc/elements/1.1/")).
  prefix(foaf: RDF::URI("http://xmlns.com/foaf/0.1/")).
  where([:s, :p, :o])

puts query.to_s
# "PREFIX {:dc=>RDF::URI(<http://purl.org/dc/elements/1.1/>)} PREFIX {:foaf=>RDF::URI(<http://xmlns.com/foaf/0.1/>)} SELECT * WHERE { ?s ?p ?o . }"

query.each_solution do |solution|
  puts solution.inspect
end

# <SPARQL::Client::MalformedQuery: Virtuoso 37000 Error SP030: SPARQL compiler, line 1: syntax error at '{' before ':dc'

# SPARQL query:
# PREFIX {:dc=>RDF::URI(<http://purl.org/dc/elements/1.1/>)} PREFIX {:foaf=>RDF::URI(<http://xmlns.com/foaf/0.1/>)} SELECT * WHERE { ?s ?p ?o . } Processing query PREFIX {:dc=>RDF::URI(<http://purl.org/dc/elements/1.1/>)} PREFIX {:foaf=>RDF::URI(<http://xmlns.com/foaf/0.1/>)} SELECT * WHERE { ?s ?p ?o . }>

Related to https://github.com/ruby-rdf/sparql-client/issues/4

ConorSheehan1 commented 5 years ago

I started working on a pr that would ensure prefixes are added as strings, but looking at https://github.com/ruby-rdf/sparql-client/issues/4 I think it was intended that prefix would take two params. An rdf uri, and a string to bind the uri to a prefix. https://github.com/ruby-rdf/sparql-client/issues/4#issuecomment-273116

Would it make sense for me to do a pr that would support that syntax?
Or is there a different approach that'd make more sense now?

gkellogg commented 5 years ago

In retrospect, I think that it's odd that prefix takes a string, rather than two separate arguments, but it's been cast in stone for some time now, not withstanding the bad example.

If anything, we could consider the following change:

##
# @overload prefix(uri, prefix)
#   @example PREFIX dc: <http://purl.org/dc/elements/1.1/> PREFIX foaf: <http://xmlns.com/foaf/0.1/> SELECT * WHERE \{ ?s ?p ?o . \}
#     query.select.
#       prefix(RDF::URI("http://purl.org/dc/elements/1.1/"), :dc).
#       prefix(RDF::URI("http://xmlns.com/foaf/0.1/"), :foaf).
#       where([:s, :p, :o])
#
#   @param [RDF::URI] uri
#   @param [Symbol, String] prefix
#   @return [Query]
#
# @overload prefix(string)
#   @example PREFIX dc: <http://purl.org/dc/elements/1.1/> PREFIX foaf: <http://xmlns.com/foaf/0.1/> SELECT * WHERE \{ ?s ?p ?o . \}
#     query.select.
#       prefix("dc: <http://purl.org/dc/elements/1.1/>").
#       prefix("foaf: <http://xmlns.com/foaf/0.1/>").
#       where([:s, :p, :o])
#
#   @param [string] string
#   @return [Query]
# @see    http://www.w3.org/TR/sparql11-query/#prefNames
def prefix(*args)
  case args.length
  when 1
    (options[:prefixes] ||= []) << string
  when 2
    (options[:prefixes] ||= []) << "#{args[2]}: #{args[1]}"
  else
    raise ArgumentError, "wrong number of arguments (#{args.length} for 1 or 2)"
  end
  self
end

We could do further sanity checking on the arguments as well.

Of course, we'd want to add some tests.

ConorSheehan1 commented 5 years ago

@gkellogg That makes sense to me, I'll start working on a pr with some tests for that format now.

Do you have any preference in terms of the argument order for the 2 argument case?
I was thinking of reversing the order since prefix(:dc, RDF::URI('some_uri')) would be closer to the string form prefix('dc: <some_uri>')

ConorSheehan1 commented 5 years ago

Maybe even something like this?

##
# @overload prefix(prefix: uri)
#   @example PREFIX dc: <http://purl.org/dc/elements/1.1/> PREFIX foaf: <http://xmlns.com/foaf/0.1/> SELECT * WHERE \{ ?s ?p ?o . \}
#     query.select.
#       prefix(dc: RDF::URI("http://purl.org/dc/elements/1.1/")).
#       prefix(foaf: RDF::URI("http://xmlns.com/foaf/0.1/")).
#       where([:s, :p, :o])
#
#   @param [RDF::URI] uri
#   @param [Symbol, String] prefix
#   @return [Query]
#
# @overload prefix(string)
#   @example PREFIX dc: <http://purl.org/dc/elements/1.1/> PREFIX foaf: <http://xmlns.com/foaf/0.1/> SELECT * WHERE \{ ?s ?p ?o . \}
#     query.select.
#       prefix("dc: <http://purl.org/dc/elements/1.1/>").
#       prefix("foaf: <http://xmlns.com/foaf/0.1/>").
#       where([:s, :p, :o])
#
#   @param [string] string
#   @return [Query]
# @see    http://www.w3.org/TR/sparql11-query/#prefNames
def prefix(val)
  if val.kind_of? String
    (options[:prefixes] ||= []) << val
  elsif val.kind_of? Hash
    options[:prefixes] ||= []
    val.each do |k, v|
      options[:prefixes] << "#{k}: <#{v}>"
    end
  else
    raise ArgumentError, "prefix must be a kind of String or a Hash"
  end
  self
end

That'd handle the case where you pass an RDF::URI as the first argument without including what prefix it should be, which would also result in a malformed query.

prefix(RDF::URI('http://xmlns.com/foaf/0.1/')).to_s
# "PREFIX http://xmlns.com/foaf/0.1/ SELECT * WHERE { }"

Do you think it'd make sense to support hashes, so something like this would work?

prefix(dc: RDF::Vocab::DC, foaf: RDF::Vocab::FOAF)

Or do you think that should always be two prefix calls?

ConorSheehan1 commented 5 years ago

I took the liberty of doing both since they're pretty similar.

91 uses the prefix(:dc, RDF::URI("http://purl.org/dc/elements/1.1/")) format

92 uses the prefix(dc: RDF::URI("http://purl.org/dc/elements/1.1/")) format

gkellogg commented 5 years ago

Closed by #92.