-
Notifications
You must be signed in to change notification settings - Fork 38
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
Format(s) of the ckan_package S3 object #113
Comments
EDIT: I have edited my original comment as, after a little more thinking, I found a better way to formulate my suggestions. My apologies for the messiness of the original comment! |
thanks for this @VLucet
|
Thank you for your reply @sckott. Replying to each of your points:
|
sorry for delay in response. I'll have a look at your README and get back to you |
@VLucet sorry again for delay, back from vacation now with time. I took a run through your README and everything looks nice. A few thoughts:
x <- govcan_search(keywords = c("dfo"), records = 2)
# methods to get metadata across every result
x$ids()
#> [1] "5cfd93bd-b3ee-4b0b-8816-33d388f6811d" "4dc95665-3d44-428c-bb26-12f981c57060"
x$
#> [1]
# methods to perform actions
x$resources() # to get resources across each package thoguhts? |
Not an urgent problem, just a suggestion on OOP in
ckanr
As I am involved in the development of the package
rgovcan
which is wrapping aroundckanr
I have been thinking about the most user-friendly way to return query results. I believe the current implementationS3
objectckan_package
could be improved to facilitate delivery to users.Currently, the output of
package_search
is a list ofckan_packages
, all with the resources returned as a list. Addingas = table
returns the results in adata.frame
(coming out offromJSON
I assume) with resources being a list ofdata.frame
s within the main resultsdata.frame
.A few suggestions:
package_search
could return a list ofckan_packages
, all with the resources returned asckan_ressource
within theckan_package
ckan_ressource
could be organised in a new classckan_ressource_stack
, and eachckan_package
returned bypackage_search
could be organised in ackan_package_stack
***So maybe what we are looking at here is switching to a S4 definition of
ckan_package
so that it contains multipleckan_ressource
. Although maybe S3 is enough for this? I am not so familaier (yet) with S4.*** For
rgovcan
, I am experimenting with attempts to write new classesckan_package_stack
andckan_ressource_stack
that stacks the output frompackage_search
with associated methods to access resources of the object, to make the querry results more user-friendly. I am implementing that in the first version ofrgovcan
because it is my first time doing OOP, but it might make more sense (and better practice) to migrate these tockanr
later on, if it is preferred!The text was updated successfully, but these errors were encountered: