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

Create meta.yaml for Spectra #160

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Create meta.yaml for Spectra #160

wants to merge 2 commits into from

Conversation

Tobiaspk
Copy link

Name of the tool: Spectra

Short description: Spectra is a supervised factor analysis method that fits a seeded matrix factorization using either gene sets or a graph.

How does the package use scverse data structures (please describe in a few sentences): All main operations, such as gene selection, filters etc. are based on adata structures.

  • The code is publicly available under an OSI-approved license
  • The package provides versioned releases
  • The package can be installed from a standard registry (e.g. PyPI, conda-forge, bioconda)
  • The package uses automated software tests and runs them via continuous integration (CI)
  • The package provides API documentation via a website or README
  • The package uses scverse datastructures where appropriate (i.e. AnnData, MuData or SpatialData and their modality-specific extensions)
  • I am an author or maintainer of the tool and agree on listing the package on the scverse website

@grst
Copy link
Contributor

grst commented Apr 18, 2024

Hi @Tobiaspk,

thanks for submitting your package!

I have a couple of comments:

The package provides versioned releases

While you do have versioned releases on PyPI, please also tag them on GitHub and/or make releases using the GitHub release feature.

The package uses automated software tests and runs them via continuous integration (CI)

The tests look mostly good, but the functions seem to only test that the model can be run and some general assertions. I was wondering if it would be possible to also test for correctness, e.g. by running the model on a small test dataset and compare against a snapshot of the results? That way you would at least be alerted if something changes.

The package provides API documentation via a website or README

By API documentation, we mean an overview of all user-facing functions/classes and documentation of their parameters. While you porvide this for est_spectra in your README and some information about some other functions, this is by no means a complete overview:

from .Spectra import SPECTRA, SPECTRA_EM, SPECTRA_Model
from .Spectra import (
    get_factor_celltypes,
    est_spectra,
    load_from_pickle,
    graph_network,
    graph_network_multiple,
    gene_set_graph,
)
from .load import default_gene_sets, sample_data

While it's possible to do this manually in the README, I'd recommend checking out the sphinx autodocs module. Here's an example of it in action. And you can check out our cookiecutter for a spinx setup.

Other comments:

  • I would get rid of the requirements.txt. It's redundant information to pyproject.toml. When you run pip install -e . it automatically resolves the dependencies. You can also declare optional dependencies like pytest in your pyproject.toml. Then you can run e.g `pip install -e ".[dev]" to install them all.
  • You seem to be recommending the scran package for preprocessing. There's also Python implementations, e.g. https://github.com/sfortma2/scranPY, https://github.com/BiocPy/scranpy - I haven't used either and you might have seen them already.

@grst grst mentioned this pull request Apr 18, 2024
@Tobiaspk
Copy link
Author

Hi Gregor, thanks for your review and feedback. We'll update both packages as soon as possible, will reach out when done!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants