Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upFix handling of fq #36
Conversation
Currently fq is completely broken: 1. It requires start to be set for no reason 2. It fails because match.arg will always fail since no args are given in the function This fix does three things: 1. Test for fq rather than start 2. get rid of the match.arg -- since fq is super flexible, we can't use that here 3. enclose the fq query in I to prevent URL encoding (see r-lib/httr#540 (comment) ). Otherwise, httr encodes colons, square brackets and + and e.g. the [example for date range search](http://guides.dataverse.org/en/latest/api/search.html#date-range-search-example) breaks
This comment has been minimized.
This comment has been minimized.
@wibeasley let me know if you need anything else on this (no particular rush on my end, but FWIW, I've been running this successfully from my fork) |
This comment has been minimized.
This comment has been minimized.
thanks @adam3smith for the nudge.I like your fix and love that you included a test. Do you mind adding a few more tests, such as
I was planning to add them, but think you're a better fit. I've never used this parameter --and didn't even know what it was when I read your PR. I am anxious to incorporate your PR and other son, before changes to the master require the PRs to merge the master's changes into the branches before they can be merged back into the master. |
This comment has been minimized.
This comment has been minimized.
Will do -- might take me a couple of days, but shouldn't be a problem. |
This comment has been minimized.
This comment has been minimized.
@wibeasley working on this -- I've added the zero hits test but I'm actually not quite clear what you're referring to with the two 3+ queries. (Note that the dataverse API actually allows for multiple fq parameters in a single query, but the package (or maybe even R?) balks at having a parameter repeated. iIf that's OK I'd like to not address that for now since it may require more substantial code changes) |
This comment has been minimized.
This comment has been minimized.
@adam3smith, (I'm looking at Advanced Search Example docs now. I've never used this, so feel free to tell me that I not making sense.)
maybe:
ahh, I didn't know that, so scratch that 2nd checkbox. Thanks for correcting me.
Sure. And thanks for adding the zero-hits test. |
This comment has been minimized.
This comment has been minimized.
got it, done -- please take a look & let me know if anything else is missing. The way I understand fq, it's mostly used for the facets available on the left, but there are some additional options such as the |
This comment has been minimized.
This comment has been minimized.
@adam3smith, looks great. Thanks again. |
This comment has been minimized.
This comment has been minimized.
If it helps, "fq" is for "filter query". The idea is that first you do a query with "q". Under the covers we pass a "*" (wildcard) as "q" when you are browsing Dataverse. Then you use "fq" to refine or limit or narrow down your original "q". That is to say, there is always a "q" but the "fq" is optional. And as you have observed, you can have as many "fq"s as you want. You can only have one "q". I hope this makes sense. You can also use boolean operators and such in both "q" and "fq" if you want. And operators like "TO" that @adam3smith is using with |
adam3smith commentedDec 21, 2019
•
edited
Currently fq is completely broken:
This fix does three things:
Please ensure the following before submitting a PR:
/R
not/man
and rundevtools::document()
to update documentation/tests
for any new functionality or bug fixR CMD check
runs without error before submitting the PR