mgirlich / tibblify

Rectangle Nested Lists
https://mgirlich.github.io/tibblify/
GNU General Public License v3.0
67 stars 2 forks source link

`parse_openapi_spec()` improvements #187

Closed mgirlich closed 1 year ago

mgirlich commented 1 year ago

Fixes #186.

Now the three specs that failed all work:

asana_yaml <- yaml::read_yaml("https://raw.githubusercontent.com/Asana/openapi/master/defs/asana_oas.yaml", handlers = list(int = as.character))
asana_spec <- parse_openapi_spec(asana_yaml)

zoom_yaml <- yaml::read_yaml("https://api.apis.guru/v2/specs/zoom.us/2.0.0/openapi.yaml", handlers = list(int = as.character))
zoom_spec <- parse_openapi_spec(zoom_yaml)

youtube_yaml <- yaml::read_yaml("https://api.apis.guru/v2/specs/googleapis.com/youtube/v3/openapi.yaml")
youtube_spec <- parse_openapi_spec(youtube_yaml)

~While this can now be parsed, the performance could be improved.~ After some minor changes the performance is quite okay.

jonthegeek commented 1 year ago

Amazing, thank you! This appears to work great!

How would you feel about including the rest of the information in the spec (perhaps subject to arguments in parse_openapi_spec)? You're parsing the returns, which is definitely useful, but I'd like to also get the arguments, descriptions, etc.

At a minimum I'll be building off what you have (and using tibblify as the format for including specifications about what to expect), but it'd be helpful if it were built in completely.

mgirlich commented 1 year ago

I'm not quite sure. It kind of makes sense as this might be quite easy to integrate. On the other hand, this doesn't really fit to tibblify. I'll have a look into this to get a better feeling for the whole issue.

jonthegeek commented 1 year ago

To be clear, I just want to include everything that fits into that tibble; to add the missing columns from the incoming spec. I'm not sure why that wouldn't fit in tibblify.

mgirlich commented 1 year ago

@jonthegeek I added a PR #191 that parses more fields of the Open API spec. Does that help you or do you already have a parser by now?

jonthegeek commented 1 year ago

@jonthegeek I added a PR #191 that parses more fields of the Open API spec. Does that help you or do you already have a parser by now?

Good timing! I haven't been focused on this part, but today I'm spending the whole day working out a formal "R API definition" class that will be the direct input for my functions in a couple different packages, so I was going to see if what you do here will still be useful, or if it makes more sense to go from scratch. I suspect I'll want to go from scratch, because I want ALL of the definition, and some things almost definitely won't be tibbles... But I'll know more in a few hours!

jonthegeek commented 1 year ago

Yeah, this looks great... but I don't think it makes sense for tibblify to cover what I'm looking for. Keep an eye on {rapid} for my R API Definition package, which will HOPEFULLY have a v0.1 in the next couple days (or at least a PR with the basic idea laid out).