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

Improve survival analysis interface #825

Merged
merged 31 commits into from
Dec 2, 2024
Merged

Improve survival analysis interface #825

merged 31 commits into from
Dec 2, 2024

Conversation

aGuyLearning
Copy link
Collaborator

@aGuyLearning aGuyLearning commented Nov 13, 2024

Description of changes
closes #822

Changed the method signature of univariate models to allow the user to pass parameters to the lifeline. Now weilbull, neil_aalen and kmf look like

adata: AnnData, duration_col: str, event_col: str, **kwargs

and are passed on to the _univariate_model function

Technical details

this could be changed to explicitly have all parameters in each function and pass them on.

Additional context

image

@eroell eroell self-requested a review November 13, 2024 14:12
@eroell
Copy link
Collaborator

eroell commented Nov 13, 2024

Thanks a lot @aGuyLearning, awesome! Comments as per review :)

model = model_class()
model.fit(T, event_observed=E)
model.fit(
Copy link
Collaborator

Choose a reason for hiding this comment

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

Add fit_left_censoring call upon argument input left

Copy link
Collaborator

Choose a reason for hiding this comment

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

check if nelson-aalen gracefully crashes with meaningful error message

ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
- removed kwargs
- updated documentation
@eroell
Copy link
Collaborator

eroell commented Nov 20, 2024

On a sidenote: having a descriptive PR title is better here - the titles end up in the changelog, forgot to mention

@eroell
Copy link
Collaborator

eroell commented Nov 20, 2024

imblearn maintenance for sklearn usage (failing test) which @aGuyLearning noted is on the way btw

@eroell eroell self-requested a review November 20, 2024 17:33
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
tests/tools/test_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Show resolved Hide resolved
ehrapy/tools/_sa.py Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
tests/tools/test_sa.py Outdated Show resolved Hide resolved
@Zethson Zethson changed the title Enhancement/issue 822 Improve survival analysis interface Nov 28, 2024
@Zethson
Copy link
Member

Zethson commented Nov 28, 2024

FYI: I merged main into this PR because I fixed the RTD build.

ehrapy/tools/_sa.py Show resolved Hide resolved
ehrapy/tools/_sa.py Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Show resolved Hide resolved
ehrapy/tools/_sa.py Show resolved Hide resolved
ehrapy/tools/_sa.py Show resolved Hide resolved
@github-actions github-actions bot added the chore label Dec 1, 2024
@eroell eroell added enhancement New feature or request breaking Breaking Changes labels Dec 1, 2024
@eroell
Copy link
Collaborator

eroell commented Dec 1, 2024

Objections to lifeline's intersphinx mappings @Zethson?
Else, once the checks run through, this is ready to merge, unless you have something to add or an earlier comment you'd want to discuss!

Copy link
Member

@Zethson Zethson left a comment

Choose a reason for hiding this comment

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

We could probably remove the kmf import in tools and just alias it but it's NBD

ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
ehrapy/tools/_sa.py Outdated Show resolved Hide resolved
@Zethson Zethson merged commit 861d762 into main Dec 2, 2024
14 of 15 checks passed
@Zethson Zethson deleted the enhancement/issue-822 branch December 2, 2024 08:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking Changes chore enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Make Survival Analysis Estimators consistent, and pass all required arguments
3 participants