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

Roverman issues #177

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open

Roverman issues #177

wants to merge 6 commits into from

Conversation

overmar
Copy link

@overmar overmar commented May 12, 2019

This should be able to close #142, though it will continue to default to 2016, which means none of the test should fail.

@overmar
Copy link
Author

overmar commented May 12, 2019

If it doesn't pass its CI, I will go back and make the correct changes.

@jackwasey
Copy link
Owner

This is a great start, thanks very much!

Would you rather have this create a whole new set of functions for ahrq_elix, or shoehorn it into the van_walraven? I liked how you gave weight arguments for the charlson function, so I am just wondering if the same should be done for the elixhauser so that when new weights are added its a drop in, rather than a wholly new function.

Not quite sure what you mean: my understanding was that you were trying to update the AHRQ comorbidity maps with optional annual revisions. This alone is great, but it seems you are thinking ahead about scoring based off this? The van walraven score function currently does not work with AHRQ comorbidities, but the original Elixhauser categories. It may well be possible to do this correctly, but I haven't given it any thought. Is this what you mean?

Other things to do will be to make sure the SAS code is indeed parsed correctly with some tests, and figure out the best way to offer the user a choice of which AHRQ year to use. I would think a year/version argument for comorbid_ahrq would be best, with the default always being the most recent data.

@overmar
Copy link
Author

overmar commented May 14, 2019

Other things to do will be to make sure the SAS code is indeed parsed correctly with some tests, and figure out the best way to offer the user a choice of which AHRQ year to use. I would think a year/version argument for comorbid_ahrq would be best, with the default always being the most recent data.

I can write in some tests to check how the values are being parsed. In this case all I can say is thank god for datapasta. Is it possible to change the _map variables in the code, rather than using the default ones? Should this possibly be analogous to the icd-cm.ver functions? In that the you aren't actually changing the package value for the map, but are rather changing which value is currently being used? Basically I am just trying to understand how to change the map values, bc this seems to be the easiest way to change the referent and therefore change its values. As far as I can tell download_all_icd_data doesn't include the codelists.

This alone is great, but it seems you are thinking ahead about scoring based off this? The van walraven score function currently does not work with AHRQ comorbidities, but the original Elixhauser categories. It may well be possible to do this correctly, but I haven't given it any thought. Is this what you mean?

That is exactly what I mean, it seems like you largely set this up with the charlson functions, but that the elix function is locked into the van walraven, though other scoring values will inevitably be published. I could envision a way to pass the map and the weights alone into the function, which would then allow researchers to include a scheme, even if it hadn't been implemented in the package. Though this probably should be put into its own issue and pr.

@overmar
Copy link
Author

overmar commented May 14, 2019

Ok I think the tests should pass, and I added a test to check to make sure that the package icd10_map_ahrq was updated. Also once I get your thoughts on the previous comment I will try to update comorbid_ahrq to take the right arguments.

@jackwasey
Copy link
Owner

I'll be getting back to this after a CRAN-requested mini-release. Thanks very much for your contribution.

jackwasey pushed a commit that referenced this pull request May 1, 2020
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.

Update AHRQ Elixhauser mappings to 2018 version
2 participants