Skip to content
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

feat: gains isic_classification Revision 5 #347

Merged
merged 10 commits into from
Mar 6, 2024
Merged

Conversation

jdhoffa
Copy link
Member

@jdhoffa jdhoffa commented Mar 6, 2024

In this PR, we update the dataset isic_classificaiton to Revision 5, rather than deprecating it entirely.
Since we are between releases, the NEWS.md was adjusted appropriately, and I just removed all deprecation messages for isic_classification entirely.

@jacobvjk I would appreciate a review of the actual content of this PR, as I visually inspected the sectors to bridge them to PACTA sectors, there is a good chance I missed some.

Note, I did this entirely functionally, so it should be pretty easy to see the logic I used to map sectors.

Relates to #329

@jdhoffa jdhoffa requested a review from jacobvjk March 6, 2024 11:30
Copy link
Member

@jacobvjk jacobvjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Their are a few cases where I would add/change borderline classifications

@jdhoffa jdhoffa requested a review from jacobvjk March 6, 2024 13:53
Copy link
Member

@jacobvjk jacobvjk left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@jdhoffa jdhoffa merged commit be3f02e into main Mar 6, 2024
24 checks passed
@jdhoffa jdhoffa deleted the 329-add_isic_rev_5 branch March 6, 2024 14:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants