optionally return results as literals #18
Conversation
elrayle
force-pushed
the
maintain_literals
branch
from
308a7c9
to
1ab3963
Compare
elrayle
force-pushed
the
maintain_literals
branch
from
1ab3963
to
c3826cb
Compare
|
Should there be docs somewhere documenting the new param and what it does? Either method-level comment docs on the methods that take the new arg, or README, or something? I had trouble figuring out what it did at first looking at the code -- is it the difference between returning a String (?) and an |
| @@ -36,7 +36,7 @@ context = RDF::Graph.new << [uri, RDF::Vocab::DC.title, "Some Title"] | |||
|
|
|||
| program = Ldpath::Program.parse my_program | |||
| output = program.evaluate uri, context: context | |||
| # => { ... } | |||
| # => {"title"=>["Some Title"]} | |||
There was a problem hiding this comment.
Added the expected result should be for documentation purposes.
There was a problem hiding this comment.
That documents the normal case, but the thing being added in this PR with maintain_literals is still (I think?) entirely undocumented. As the simplest possible thing, since ldpath doesn't seem super documented in general anyway but that's not a problem for this PR, what about just adding antoher examples to the README with maintain_literals: true, and what that would return?
Also, just confirm you think maintain_literals is the right name of this arg? Preferable to, oh, use_literals or return_literals or return_rdf_literals or rdf_literals: true or something like that? I think it's up to you, I just think it's good to at least publicly consider if it's the best one we can think of before locking it into a release from which it can't be changed without backwards incompat.
I'm not totally locked into any of these ideas, just trying my best to give a responsible non-rubberstamping review on code I'm not actually at all familiar with. Disagreement welcome you can just tell me "nah, I know this code better and it's right how it is", and I'll just approve! Thanks!
DEFAULT remains returning results as string or the specified data type Also... * remove pin of bundler * add example results to README
elrayle
force-pushed
the
maintain_literals
branch
from
ef406b9
to
36e0706
Compare
| else | ||
| v | ||
| end | ||
| end | ||
|
|
||
| def same_type(object, field_type) |
There was a problem hiding this comment.
Should this be same_type? Should it be a private method?
Fixes #16 Optionally have ldpath return RDF::Literals instead of Strings
DEFAULT remains returning results as string or the specified data type
Also...
* remove pin of bundler
* add example results to README