-
Notifications
You must be signed in to change notification settings - Fork 6
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
All sector_classification
datasets have no duplicate values of code
column
#327
Conversation
Looking into this now, and there are some issues: |
What happens is that This is almost certainly a bug, and should be reverted, however, we do risk affecting the downstream user as it will change the expected sector classification code. I leave it to @jacobvjk and George to determine how risky this change will end up being: |
|
This is ready for review. Progress on the deprecation of |
…CTA/r2dii.data into 229-remove_duplicate_sector_codes
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.
There seems to be some stray code that I could not see used anywhere else in the repo. See comment. Maybe remove?
data-raw/classification_bridge.R
Outdated
@@ -2,15 +2,51 @@ library(usethis) | |||
|
|||
source(file.path("data-raw", "utils.R")) | |||
|
|||
letter_to_number <- Vectorize(letter_to_number) |
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.
this does not seem to be used anywhere
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.
Nice catch!
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.
lgtm
Closes #229