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

Documentation #279

Merged
merged 67 commits into from
Apr 25, 2022
Merged

Documentation #279

merged 67 commits into from
Apr 25, 2022

Conversation

mumichae
Copy link
Collaborator

@mumichae mumichae commented Dec 2, 2021

Update docstrings and create sphinx docs for readthedocs.
Rendered documentation: https://scib--279.org.readthedocs.build/en/279/ (from in github checks at the bottom of this page)

@mumichae mumichae requested a review from LuckyMD December 2, 2021 15:27
scib/metrics/clustering.py Outdated Show resolved Hide resolved
@mumichae mumichae requested a review from LuckyMD March 10, 2022 11:08
@mumichae mumichae requested a review from lazappi March 16, 2022 14:20
@mumichae
Copy link
Collaborator Author

@LuckyMD

Can we add default values to the documentation? => mainly important for k0 in LISI scores

Afaik, default values are already included in the autodoc. Or do you want more specific explanations on which values to use?

@LuckyMD
Copy link
Collaborator

LuckyMD commented Apr 21, 2022

Afaik, default values are already included in the autodoc. Or do you want more specific explanations on which values to use?

I didn't see any in the readthedocs link...

@mumichae
Copy link
Collaborator Author

Afaik, default values are already included in the autodoc. Or do you want more specific explanations on which values to use?

I didn't see any in the readthedocs link...

Screenshot from 2022-04-22 15-04-43

Default values are shown in the function signature.

@LuckyMD
Copy link
Collaborator

LuckyMD commented Apr 22, 2022

Ah, I was looking at the parameter explanation. Okay,all good. Thanks!

Copy link
Member

@ivirshup ivirshup left a comment

Choose a reason for hiding this comment

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

Looks fantastic! A few minor points, but I think it would be good to merge.


A few general minor things which would be nice, but could/ should be separate PRs:

  • Type information + autodoc-typehints would be nice
  • It would be great if you can installation instructions for setting up an isolated environment (probably conda), especially due to the mixed language dependencies
    • Kinda related: wouldn't you need Seurat for read_seurat?
  • I would strongly recommend turning on PR builds for docs: https://docs.readthedocs.io/en/stable/pull-requests.html

scib/metrics/__init__.py Outdated Show resolved Hide resolved
scib/metrics/__init__.py Outdated Show resolved Hide resolved
docs/source/scib_metrics.rst Outdated Show resolved Hide resolved
docs/source/scib_metrics.rst Outdated Show resolved Hide resolved
@mumichae mumichae merged commit c99ebad into main Apr 25, 2022
@mumichae mumichae deleted the document branch April 25, 2022 14:13
@mumichae mumichae restored the document branch April 25, 2022 14:40
@mumichae mumichae deleted the document branch April 25, 2022 15:49
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.

4 participants