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

Repo structure #24

Closed
cheginit opened this issue Jan 19, 2024 · 7 comments · Fixed by #107
Closed

Repo structure #24

cheginit opened this issue Jan 19, 2024 · 7 comments · Fixed by #107
Assignees
Labels
infrastructure Modification or update of tooling for QA, installation, CI or deployment

Comments

@cheginit
Copy link
Collaborator

There's no consensus on what is considered a "good" structure for a python package repo. So, it's subjective, but there are some best practices. The structure of the repo is fine. Here are my main suggestions based on my experience and preference to improve it:

  • Add CONTRIBUTING, AUTHORS, CODE_OF_CONDUCT, HISTORY, and LICENSE files. There are several templates for these. So you can easily just pick one and edit based on your repo. For example, you can check out these files in PyNHD. You can move all the dev stuff that you currently have in the README to CONTRIBUTING. This will declutter the README and you have more room for other stuff.
  • Since this package depends on geospatial packages, using conda (mamba, or micromamba) is recommended for development and users, too. There are many discussions about this on the internet, but the main reason is that many of the these geospatial packages depend on non-python libraries that can be tricky to install on different platforms. Using conda solves this issue since conda packages are pre-compiled and optimized for different platforms. So, it makes managing the dependencies easier and codes may even run faster, too.
  • I highly recommend using nox for testing, linting, and type checking.

I can help with setting these up.

@barneydobson
Copy link
Collaborator

barneydobson commented Jan 19, 2024

Happy on those first two points. (Happy as in agreed)

For testing/linting/type checking - I believe the template we are using from ICL comes with a lot of this in place already - but I will have to defer to @dalonsoa on that

@barneydobson barneydobson added the infrastructure Modification or update of tooling for QA, installation, CI or deployment label Jan 19, 2024
@cheginit
Copy link
Collaborator Author

Nox does not replace what you have, it just streamlines the development. For example, instead of running the linter, type checker, and pytest with different commands, you can setup nox and just type in nox in the command-line and will run all these commands one by one and create environments for each, clean up stuff, etc.

@dalonsoa
Copy link
Collaborator

Totally supportive of 1.

Agree with 2 if non-python stuff is absolutely necessary (otherwise normal python is totally fine), but will go for mamba or micromamba. Anaconda and relatives are an absolute pain to work with. They are very heavy, slow and unreliable.

I've just used nox just a couple of times and I feel it overcomplicates things quite a bit. We tent to use pre-commit for linting, formatting, type checking, etc. and directly pytest for testing. Granted that you don't test for multiple python versions locally, but that's what GitHub Actions is there for.

@cheginit
Copy link
Collaborator Author

Regarding nox, I agree that it has a learning curve, but its handling of environments for each task separately and effortlessly is very useful, even if you locally test your code with one python version.

@dalonsoa
Copy link
Collaborator

My bad: I did not try nox but tox, which is way more complicated.

I've just had a look at nox and it looks pretty nice and easy to use. I still think that, unless you're trying things on multiple python versions or dependencies versions, it is a bit overkilling and just running pytest and pre-commit is good enough (pre-commit also run its stuff in independent environments), but it is certainly an easy enough to learn tool and convenient in many cases.

In summary: choose one tool and stick to it :)

@cheginit
Copy link
Collaborator Author

IMHO, the biggest advantage of using nox is using a separate environment than the dev for testing. I use micromamba for managing envs and, often, in a project's dev env, I install optional dependencies and other packages that are not necessarily hard deps of the project. Also, I usually use the latest Python version for development, but I want to run tests with the min Python version that the project is supposed to support. So, with nox I don't have to worry about this type of stuff since you can create different envs for each step and strictly follow deps based on the pyproject.toml file.

I have one test env and a basenoxfile.py that I use and modify for running nox in all my projects. Here's the base noxfile.py:

"""Nox sessions."""
from __future__ import annotations

import shutil
from pathlib import Path

import nox

try:
    import tomllib as tomli
except ImportError:
    import tomli


def get_deps() -> list[str]:
    """Get the main deps."""
    with Path("pyproject.toml").open("rb") as f:
        return tomli.load(f)["project"]["dependencies"]


def get_extras() -> list[str]:
    """Get the extra deps."""
    with Path("pyproject.toml").open("rb") as f:
        extras = tomli.load(f)["project"]["optional-dependencies"]
    return [e for e in extras if e not in ("test", "doc")]


test_version = ["3.9"]
lint_version = ["3.11"]
nox.options.sessions = (
    "pre-commit",
    "type-check",
    "tests",
)


def install_deps(session: nox.Session, extra: str | None = None) -> None:
    """Install package dependencies."""
    deps = [f".[{extra}]"] if extra else ["."]
    session.install(*deps)
    dirs = [".pytest_cache", "build", "dist", ".eggs"]
    for d in dirs:
        shutil.rmtree(d, ignore_errors=True)

    patterns = ["*.egg-info", "*.egg", "*.pyc", "*~", "**/__pycache__"]
    for p in patterns:
        for f in Path.cwd().rglob(p):
            shutil.rmtree(f, ignore_errors=True)


@nox.session(name="pre-commit", python=lint_version)
def pre_commit(session: nox.Session) -> None:
    """Lint using pre-commit."""
    session.install("pre-commit")
    session.run(
        "pre-commit",
        "run",
        "--all-files",
        "--hook-stage=manual",
        *session.posargs,
    )


@nox.session(name="type-check", python=test_version)
def type_check(session: nox.Session) -> None:
    """Run Pyright."""
    extras = get_extras()
    install_deps(session, ",".join(extras))
    session.install("pyright")
    session.run("pyright")


@nox.session(python=test_version)
def tests(session: nox.Session) -> None:
    """Run the test suite."""
    install_deps(session, "test")
    session.run("pytest", "--doctest-modules", *session.posargs)
    session.notify("cover")


@nox.session
def cover(session: nox.Session) -> None:
    """Coverage analysis."""
    session.install("coverage[toml]")
    session.run("coverage", "report")
    session.run("coverage", "erase")

@dalonsoa
Copy link
Collaborator

This is really good stuff. Very powerful and definitely worth a try.

Having said that is a tool on top of pytest, pre-commit and coverage (in your particular example), so something else to worry about and that might go wrong. But, as I said, I will try it because it looks pretty nice.

@barneydobson barneydobson self-assigned this Mar 21, 2024
@barneydobson barneydobson mentioned this issue Mar 21, 2024
6 tasks
@barneydobson barneydobson linked a pull request Mar 21, 2024 that will close this issue
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infrastructure Modification or update of tooling for QA, installation, CI or deployment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants