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

simplify.sparqlResults has multiple possible result types #45

Closed
EdJoPaTo opened this issue Mar 13, 2019 · 5 comments
Closed

simplify.sparqlResults has multiple possible result types #45

EdJoPaTo opened this issue Mar 13, 2019 · 5 comments

Comments

@EdJoPaTo
Copy link
Contributor

I would like something opposite to #40: Do not simplify sparql results as much as its currently done and generalise its output.

Simplifying the query (removing not needed variables from the select) results in a different result from simplify.sparqlResults.
Currently the result can be either an array of objects with the variables as keys or just an array of the single variable specified in the query.

(Whats actually worse: undefined entries in the array get lost. But when the behaviour is changed this bug will be gone too.)

When the user wants the simple array of the single variable its easy to use something like .map(o => o.amountChildren). Also another simplify method could be doing that.

This would definitely be a breaking change.

Example query (query.wikidata.org):

SELECT DISTINCT ?amountChildren WHERE {
  ?item wdt:P31 wd:Q5.
  OPTIONAL { ?item wdt:P1971 ?amountChildren. }
}
LIMIT 5

The result of the simplify.sparqlResults will be (something like): [2, 6, 0, 1]

When the query is modified to return a second variable like ?item it will return (something like) this:

[
  {amountChildren: 6, item: 'Q1785'},
  {amountChildren: 2, item: 'Q2010'},
  {amountChildren: undefined, item: 'Q1747'},
  {amountChildren: undefined, item: 'Q1745'},
  {amountChildren: undefined, item: 'Q1750'}
]
@maxlath
Copy link
Owner

maxlath commented Mar 13, 2019

would that do the trick to have this behavior behind a flag?

simplify.sparqlResults(results, { minimize: false })

that could maybe become the default in the future, but that would indeed be a breaking change

@EdJoPaTo
Copy link
Contributor Author

EdJoPaTo commented Mar 13, 2019

Living without it is ok for me short term.
Long term I would like a more clean usage there.

Maybe create a staging for breaking changes or something like that so it can be just merged when a new major release is ready?

Until then the bug be fixed: undefined should still be in the array.

@maxlath
Copy link
Owner

maxlath commented Mar 17, 2019

what if the flag was inverted:

  • simplify.sparqlResults(results) always returns an array of objects
  • for single variable queries, simplify.sparqlResults(results, { minimize: true }) returns an array of the values

would that work for you? I implemented that in ddba9e2, waiting for your comment to make it part of the next major version

@EdJoPaTo
Copy link
Contributor Author

I kind of like the idea but I don't think its perfect. As I tried to add typescript types (#50) I wanted to have as strict types as possible. An option determining the output of the method still has a less strict output then.

With the option as argument you depend on the input to know the output. For other languages like Java, C#, … this would require overloading the method.

I thought about having two methods for that. One with the always object array behaviour. The other one always returns simple value array and fails when it has more than one value per entry. That way the same "overloading" is there but with strictly separated names. But the "its basically the same so same method" idea is lost then.

Not sure what would be best…

@EdJoPaTo
Copy link
Contributor Author

Looks like you went with the minimize way for 6.0.0.

I adapted #50 accordingly in d52f6b8.

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

2 participants