-
Notifications
You must be signed in to change notification settings - Fork 0
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
New feature - Translate natural geographic level naming to API abbreviations on query input #70
Conversation
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.
Tests themselves all pass, but there's an issue with the vignette that you need to fix (is causing all the CI failures)
Slightly surprised that this was the bit you did, would have thought having National in the output would be more helpful than in the inputs for the query, though happy for this to go in and to do the other half later (once the vignette is fixed)
I'm just working through using eesyapi for the attendance dashboard, so dealing with things in the order I hit them in the implementation. This was prompted by linking the API call to the inputs, which meant having to write something in the app or eesyapi to convert National / Regional / Local authority to NAT / REG / LA. More efficient to do just it in eesyapi now, rather than in the app now and then eesyapi later down the line. |
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.
One of my tests is failing locally but I think that's just an EES dev issue.
There's one tiny typo I've spotted and made a suggestion for though happy to approve and let you sort that one out as you merge it in
Co-authored-by: Cam Race <[email protected]>
Brief overview of changes
I've added a draft look-up table for translating natural geographic level names (e.g. National) to the API versions (e.g. NAT) and I've applied a screening on the query input to translate from the former to the latter.
This lays some groundwork for translations at the other end on the query results, but saving that for later.
Why are these changes being made?
The API is quite prescriptive when it comes to providing geographic levels and this is intended to give users a more flexible / forgiving experience.
Detailed description of changes
So that I'm not repeating code, I've stuck this in todf_geographies(), which feeds into
post_dataset()
andget_dataset()
Additional information for reviewers
Issue ticket number/s and link
Partial groundwork for some of #5