Skip to content

ENH allow initializing a DeseqDataSet from an AnnData object#96

Merged
BorisMuzellec merged 6 commits intomainfrom
dds_from_adata
Mar 15, 2023
Merged

ENH allow initializing a DeseqDataSet from an AnnData object#96
BorisMuzellec merged 6 commits intomainfrom
dds_from_adata

Conversation

@BorisMuzellec
Copy link
Copy Markdown
Collaborator

@BorisMuzellec BorisMuzellec commented Mar 7, 2023

Reference Issue or PRs

Fixes #94

What does your PR implement? Be specific.

This PR provides the option to initialize an DeseqDataSet by passing an AnnData as an argument.

@BorisMuzellec BorisMuzellec marked this pull request as ready for review March 10, 2023 14:35
@BorisMuzellec BorisMuzellec requested a review from a user March 10, 2023 14:35
Comment on lines +160 to +161
counts: Optional[pd.DataFrame] = None,
clinical: Optional[pd.DataFrame] = None,
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

You may want to make those keyword-only arguments. Also all the configuration arguments that come after are unlikely to be called by position. Making them keyword-only saves you from breaking API compatibility should you decide to add more or reorder them in the future.

For counts/clinical, another option would be to remove them entirely. After all, it's easy to do

adata = AnnData(counts, obs=clinical)

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Thank for this helpful suggestion. I have made all arguments keyword-only.

Copy link
Copy Markdown
Collaborator

@maikia maikia left a comment

Choose a reason for hiding this comment

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

LGTM. Perhaps we could also start a CHANGELOG (it could already be included within this PR).
This change could potentially cause errors in existing scripts if the version is being updated

@BorisMuzellec
Copy link
Copy Markdown
Collaborator Author

BorisMuzellec commented Mar 15, 2023

Thanks! I'm merging this PR.

Perhaps we could also start a CHANGELOG (it could already be included within this PR).

@maikia there is already a changelog being written at each new PyPI release, accessible through the github tags. Do you think this enough, or should we create a dedicate file?

@BorisMuzellec BorisMuzellec merged commit 460dded into main Mar 15, 2023
@BorisMuzellec BorisMuzellec deleted the dds_from_adata branch March 15, 2023 13:42
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.

Initialize DeseqDataSet from existing AnnData object

3 participants