-
Notifications
You must be signed in to change notification settings - Fork 12
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
added xronos to getter functions #150
base: master
Are you sure you want to change the base?
Conversation
site,xronos,site | ||
,xronos,site_phase | ||
feature,xronos,feature | ||
,xronos,period |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wouldn't that fit to c14bazAAR.period
, which is defined as
Historico-cultural period of sample context
and usually contains values like "Bronze Age" etc.
,xronos,site_phase | ||
feature,xronos,feature | ||
,xronos,period | ||
,xronos,typochronological_unit |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That should fit to c14bazAAR.culture
, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
btw. the variable definition table is available here: https://github.com/ropensci/c14bazAAR/blob/master/data-raw/variable_definition.csv
R/get_xronos.R
Outdated
#' @export | ||
get_xronos <- function(db_url = get_db_url("xronos")) { | ||
|
||
check_connection_to_url(db_url) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Does this already download the whole database, because db_url
points to the endpoint at https://xronos.ch/api/v1/data/
? It takes forever. Probably it's sufficient to check if https://www.xronos.ch
is up here. Or maybe the API has a more minimal endpoint to check if it's available.
R/get_xronos.R
Outdated
|
||
xronos_data <- jsonRespParsed %>% tidyr::tibble() %>% | ||
tidyr::unnest_wider(1) %>% | ||
tidyr::unnest_wider(measurement) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These procedures take a significant amount of time and could be avoided with a simple flat file download.... 😅
@nevrome @MartinHinz We now finally provide a flat file with all our data (https://xronos.ch/data.csv; see xronos-ch/xronos.rails#115) – perhaps time to revisit this? |
Neat! Would be easy to do this now, I guess. Cool that you decided to include the flat-file version 🥳 |
Probably it would be easier to create a new PR for this. This one is a bit dated... |
…d "data." any longer?
Against your advice, I’ve decided to proceed with this PR and kindly request a new review and potential merge. Given the size of the database, transferring and parsing the 350,000 dates is still taking some time, but we anticipate improvements in the near future. Here’s the current performance benchmark:
Please note, this isn’t strictly a flat file but rather a database dump from a materialized view. I hope this approach now meets your requirements! Looking forward to your feedback. |
time to give back ;-)