-
Notifications
You must be signed in to change notification settings - Fork 0
Conversation
Hi @antschum, It looks like this pull-request is has been made against the theislab/augurpy You do not need to close this PR, you can change the target branch to Thanks again for your contribution! |
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.
Cool to see it coming together!
I'd suggest that you rename the Python files to ensure that they are descriptive.
calculate_AUC.py
for example is not the nicest of all names.
Besides that just many tiny requests by me. Happy to see this merge soon! Then we'll also need to ensure that we properly document all of this. See #40
Mhmm. Didn't we want to look at how Augur calculates them? Did you figure something out? |
Co-authored-by: Lukas Heumos <lukas.heumos@posteo.net>
Co-authored-by: Lukas Heumos <lukas.heumos@posteo.net>
I guess that this should help determine which genes contributed the most to the cell type prioritization? |
@antschum Laptop working again? ^_^ |
Hey:) using my brothers, should have a working one by Thursday though.
That would be great! Ill click on request. |
@antschum think you missed two comments of mine. (You can scroll through the changed files.) But besides this it looks great! Feel free to merge it after you've addressed my comments. Regarding the feature importances:
We can just make use of something like this, no? Also, we will need to implement some plots for the feature importances. Would you mind creating an issue for this? |
Ah! Im sorry, I totally missed those. I changed the variable name and replied to the other comment.
So would you suggest just using the coefficient values as feature importances for the logistic regression? In the original they referenced the agresti method here. I have not really found much on this, although in the blog post, it seems they are just subtracting the mean from each coefficient and then dividing by the standard deviation, right?
Done! #42 |
Ah, wasn't aware of this. Yes, I read the blog the same way. An alternative comparison of effects of quantitative predictors having different units uses standardized coefficients. The model is fitted to standardized predictors, replacing each xj by (xj−x¯j)/sxj. A 1-unit change in the standardized predictor is a standard deviation change in the original predictor. Then, each regression coefficient represents the effect of a standard deviation change in a predictor, controlling for the other variables. The standardized estimate for predictor xj is the unstandardized estimate β^j multiplied by sxj. Feel free to merge it now :) |
Ah, I just read this and had already pushed - maybe you could take a quick look? I added some rough tests for the concordance correlation coefficient, so it should be more or less okay. And then we merge? My bad. |
Forgot to run nox. 🤦♀️ |
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!
Description of changes
Implementation to calculate the AUC with respect to CellType along with test files.
Still missing right now: