MannLabs / scPortrait

https://mannlabs.github.io/scPortrait/
Apache License 2.0
34 stars 1 forks source link

Coding best practices #43

Open Zethson opened 4 months ago

Zethson commented 4 months ago

This is a totally random list of things that are currently not best practice in this repository that we eventually want to resolve with #27 by fixing #17 .

  1. The package has too many dependencies and some of them are difficult to install. Ideally we don't depend on users to install packages in a specific order nor having to use Conda

To resolve this we should double check every dependency. Do we really need it and can we get rid of it? Why are there installation issues for some of them on different platforms? Is there a way to get rid of all Conda dependencies? If not, can we create a conda package for sparcs?

  1. The package should be trivially installable.

Get it on PyPI and Conda

  1. Separate CLI and library. This brings a lot of complexity and weird code structure. Don't do it.

We create sparcs and sparcs-cli. Eventually both of them are on PyPI and Conda. The CLI package has sparcs and optionally click/whatever as dependencies. This helps us resolve weird code structure

  1. Doc structure is off.

There should be a single docs folder with potentially a source folder. We'll just use what the scverse templates provides us with.

  1. Code formatting and docstyles are off.

We'll use the Ruff + pre-commit setup of the scverse template. We'll use Google docstring style and configure Ruff to check it.

  1. setup.py is legacy

Adopt the pyproject.toml setup from the template. The files defined in the MANIFEST should be picked up and shipped automatically. However, larger files should potentially be downloaded lazily on request.

  1. The README is overwhelming.

Keep it minimal and remove the complex installation instructions. We'll just make it so easy so that this won't be necessary anymore Ha!

  1. No unit tests.

We should have unit tests so that at least every public function is tested. An integration test for the whole pipeline would also be good.

sophiamaedler commented 4 months ago

This is the priority order in which I would try and tackle these:

High Priority

Medium Priority

Lowest Priority

sophiamaedler commented 2 weeks ago

documentation was automated and moved to correct location in #98