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

Incorporating the latest round of API changes #39

Open
wants to merge 52 commits into
base: master
Choose a base branch
from

Conversation

mustberuss
Copy link
Collaborator

Things to note:

  • The new API field shorthand is allowed in the fields parameter and is used by get_fields(). Without it the patent endpoint has so many attributes that GET requests can fail (> 4K url) on some queries (if all fields were fully qualified). From their jupyter notebook:

The fields for related entities can be requested in the API request's fields parameter as a group by using the group name in the fields parameter, or individually by specifying the required field as "{entity_type}.{subfield}".

  • It's still test-heavy for now, at lot has changed and the API has problems (see test-api-bugs.R)

Should we bump the version to 1.0.0 because of the breaking changes?

@crew102
Copy link
Collaborator

crew102 commented Dec 23, 2024

Hey Russ, can you summarize what this PR does? Also I see that the tests on CI aren't passing, can you comment and/or make sure they pass?

@crew102
Copy link
Collaborator

crew102 commented Dec 23, 2024

Can we also drop all those tests that we talked about?

@mustberuss
Copy link
Collaborator Author

It's the remaining code to catch up with the API:

  • paging by primary key only and sorting in R; deprecating the page and per_page search_pv() parameters; padding the patent_id's after value when patent_id is used as the sort field; exporting the padding function and adding after as a search_pv parameter for custom paging; removal of paging limits
  • accepting group names in the field parameter (API's new shorthand); get_fields() returning group names as fields rather than fully qualifying (group.member notation) each member of the group
  • unnesting and casting changes now that the endpoint names (singular, some nested) and returned entities (mostly plural) aren't interchangeable; dealing with the two endpoints that return rel_app_texts (from patent/rel_app_text and publication/rel_app_text)
  • switching to httr2, httr says it's superseded: only changes necessary to keep it on CRAN will be made.
  • there's an additional inclulde_pk boolean on get_fields() and encoded_url boolean on retrieve_linked_data()
  • there is a new qry_fun in_range() that isn't strictly needed, it's not fronting anything in the API but would generate the tedious _and/_gte/_lte needed ex: qry_funs$in_range(patent_date = c("1976-01-01", "1983-02-28"))
  • field name/endpoint name changes in the examples
  • bug fix on posts when fields or sort wasn't specified
  • bug fix when there was more than a primary sort
  • bug fix to avoid a coercion warning

We can remove tests but some of the suggestions you made have code changes now, especially get_fields() and, argument changes to search_pv()

I'm not sure what's wrong with the build. I bumped versions in the workflow to get rid of deprecated warnings but didn't intentionally change anything, I can try reverting. It's working in my repo but it's trying to make API calls here. There are skip_on_cran()s in the tests but they're running and so are the dont_run examples.

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

Successfully merging this pull request may close these issues.

2 participants