Allow including 0% threshold

[ck37]

Hello,

Thanks for the very useful package. I'm using it for some clinical risk modeling and two changes were helpful for me: 1) allow removing the default all or none strategies, and 2) don't actively exclude the 0% threshold. Here is a quick PR for these two edits.

Cheers,
Chris

[ck37]

Hi @ddsjoberg, any reaction to this?

Thanks,
Chris

[ddsjoberg]

hello @ck37 ! Thank you for following up!

I think this PR was lost in a frenzy of github notifications. I"ll take a look in the next couple of weeks!

FYI, while I am confident in the results you've obtained using the package, the package is currently undergoing a thorough code review prior to its submission to CRAN.

[ddsjoberg]

@ck37 at first glance, I think I may prefer to include the options of which strategies to include in the plot method. The treat all strategy is required to calculate the net interventions. Anyway, i'll think on it!

[ck37]

Ah excellent point, and for my use case I only wanted to remove the "none" strategy from the plots.

[ddsjoberg]

I've worked with DCA for years, and I've never seen or heard of a request to not plot the treat all or treat none line on the figure. Can you give some background on why the treat none line should be omitted from your figure? Thanks! @ck37

[ck37]

Well, it's just a flat line at zero, so plotting "treat none" is not really informative or necessary. In my case I primarily wanted a shorter legend because I had 7 models and "treat all" to include. Here is an example paper that also doesn't plot "treat none": https://www.ahajournals.org/doi/full/10.1161/JAHA.120.020082

[ck37]

Ok simplifying this PR to just allow calculating and plotting the 0% threshold

[ddsjoberg]

@ck37 thanks for the update! I agree it's better to address these two questions separately.

Can you confirm a couple of things?
I'm away from a computer until Wednesday ⛱️🌊🏖️, so I can't check at the moment.

1. At a threshold of 0, does the treat none line have a calculated net benefits of 1 or 0? We may need to code to handle that case.
2. The default threshold values should also be updated to include 0. Thoughts?

[ck37]

Excellent points. Yep treat none was switching to a standardized net benefit of 1 at the 0% threshold, so I added a line to fix that. Updated the default thresholds to include 0 as well.

And no rush at all on this, it's a good time to take a break.

[ddsjoberg]

@ck37 oy! Apologies, I was playing around, and I accidentally pushed my changes here....I'll attempt a revert soon!

[ck37]

@ddsjoberg no worries at all, please do whatever is easiest/best for you. I realize that my edits may also not be the cleanest (in part due to my own laziness) so no pressure on my end to use.

[ddsjoberg]

the edits you made are great! the only bit remaining to figure out is how to treat binary decision models, e.g. some models/markers are binary like family history of cancer. These models are passed as 0/1, but the way we have the thresholds now that include 0, the NB for those models shoots to 1 (for standardaized NB) or the prevalence. So the issue wasn't isolated to the treat none strategy.

I am thinking the best/easiest move may be to just replace thresholds of 0 with 10e-10?

[ddsjoberg]

@ck37 hey hey!

I think this should be ready to go. Can you take a quick pass at the changes I made before we merge?

[ck37]

@ddsjoberg looks great to me. I also reran it on my data and it works like a charm.

[ddsjoberg]

great, thank you so much! I love that the figures start at zero now (or t least look like they do!)