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

DX: implement developer infrastructure #1

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

DX: implement developer infrastructure #1

wants to merge 1 commit into from

Conversation

Zeyna777
Copy link
Contributor

@Zeyna777 Zeyna777 commented Aug 22, 2024

Closes #2

@Zeyna777 Zeyna777 added the 🖱️ DX Improvements to the Developer Experience label Aug 22, 2024
@Zeyna777 Zeyna777 requested a review from redeboer August 22, 2024 11:52
@redeboer redeboer assigned Zeyna777 and unassigned redeboer Aug 22, 2024
Copy link
Member

@redeboer redeboer left a comment

Choose a reason for hiding this comment

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

See #2

- --no-pypi
- --no-version-branches
- --pin-requirements=monthly
- --repo-name=K-matrix-research
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- --repo-name=K-matrix-research
- --repo-name=kmatrix

hooks:
- id: check-dev-files
args:
- --dev-python-version=3.10
Copy link
Member

Choose a reason for hiding this comment

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

Let's use the newest here:

Suggested change
- --dev-python-version=3.10
- --dev-python-version=3.12

metadata.vscode

- repo: https://github.com/ComPWA/policy
rev: 0.3.5
Copy link
Member

Choose a reason for hiding this comment

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

Please run

pre-commit autoupdate

Comment on lines +25 to +37
dependencies = [
"ampform>=0.14.10",
"ipympl",
"ipywidgets",
"matplotlib",
"mpl-interactions",
"orthopy",
"pandas",
"pyarrow",
"qrules[viz]",
"tensorwaves[jax,phsp]",
"tqdm",
]
Copy link
Member

Choose a reason for hiding this comment

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

The philosophy: dependencies contain the packages that your package ('kmatrix' — the stuff under src/) requires to run.
Further down, you have a table with optional-dependencies that define dependencies required to do specific tasks, such as building documentation (doc), running tests (test) and developer CLI tools (dev), the actual developer environment that also contains all other categories).

All of these you want to keep as limited as possible, only listing what you really need. For now, dependencies can be left empty, while optional-dependencies should contain developer tools like pre-commit.

Copy link
Member

Choose a reason for hiding this comment

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

As a side note, optional-dependencies are exposed to users installing your package (e.g. via PyPI) and are therefore actually intended for additional features your package may want to offer (like different backends).

Unfortunately, Python does not yet have a mechanism for specifying dependencies that are only for developers and are not to be exposed to users of the package. There have been big debates on this and each package manager has its own way of doing this (see e.g. tool.uv.dev-dependencies). The latest attempt to standardise Python dev dependencies is PEP 735, but it is still being drafted.

Comment on lines +38 to +43
- --no-github-actions
- --no-gitpod
- --no-prettierrc
- --no-pypi
- --no-version-branches
- --pin-requirements=monthly
Copy link
Member

Choose a reason for hiding this comment

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

In this public repo, we can activate all CI workflows

Suggested change
- --no-github-actions
- --no-gitpod
- --no-prettierrc
- --no-pypi
- --no-version-branches
- --pin-requirements=monthly
- --no-gitpod
- --no-prettierrc
- --pin-requirements=monthly

Comment on lines +222 to +308
[tool.tox]
legacy_tox_ini = """
[tox]
envlist =
doc,
skip_install = True
skip_missing_interpreters = True
skipsdist = True

[testenv:doc]
allowlist_externals =
sphinx-build
commands =
sphinx-build \
--keep-going \
-TW \
-b html \
docs/ docs/_build/html
description =
Build documentation with Sphinx
passenv = *
setenv =
FORCE_COLOR = yes

[testenv:doclive]
allowlist_externals =
sphinx-autobuild
commands =
sphinx-autobuild \
--open-browser \
--re-ignore docs/api/.* \
--re-ignore docs/_build/.* \
--watch docs \
docs/ docs/_build/html
description =
Set up a server to directly preview changes to the HTML pages
passenv = *
setenv =
FORCE_COLOR = yes

[testenv:docnb]
allowlist_externals =
sphinx-build
commands =
sphinx-build \
--keep-going \
-TW \
-b html \
docs/ docs/_build/html
description =
Build documentation with Sphinx
passenv = *
setenv =
EXECUTE_NB = yes
FORCE_COLOR = yes

[testenv:docnblive]
allowlist_externals =
sphinx-autobuild
commands =
sphinx-autobuild \
--open-browser \
--re-ignore docs/api/.* \
--re-ignore docs/_build/.* \
--watch docs \
docs/ docs/_build/html
description =
Set up a server to directly preview changes to the HTML pages
passenv = *
setenv =
EXECUTE_NB = yes
FORCE_COLOR = yes

[testenv:linkcheck]
allowlist_externals =
sphinx-build
commands =
sphinx-build \
-T \
-b linkcheck \
docs/ docs/_build/linkcheck
description =
Check external links in the documentation (requires internet connection)
passenv = *
setenv =
FORCE_COLOR = yes
"""
Copy link
Member

@redeboer redeboer Aug 22, 2024

Choose a reason for hiding this comment

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

I'm still not really sure which package manager we want to use. Currently, this repo suggests Conda (environment.yml), but it has the disadvantage that (1) it is slow and (2) it does not have a mechanism to define standardized tasks (currently done through tox).

A fast alternative that has is being used with success in ComPWA/gluex-nstar and RUB-EP1/amplitude-serialization is Pixi. It's fast, can define tasks and has cross-platform dependency lock files. Disadvantages are that it is relatively new, not specifically designed for Python, and results in an unstable lockfile if the package is installed in editable mode..

Finally, there is uv, which is specific for Python. It's exciting that it has recently added cross-platform dependency resolution as well bootstrapping of multiple Python versions (removing the need for Conda on that front). Tasks would still have to be defined through tox though.


All an all, I would ditch Conda for either Pixi or for uv. Perhaps Pixi is more convenient, because we also need developers to install graphviz, which uv can't do.

Copy link
Member

Choose a reason for hiding this comment

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

Vote for pixi

Copy link
Member

Choose a reason for hiding this comment

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

@redeboer redeboer changed the title dev infrastructure DX: implement developer infrastructure Aug 22, 2024
@redeboer redeboer requested a review from shenvitor August 23, 2024 12:11
hooks:
- id: nbstripout
args:
- --extra-keys
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- --extra-keys
- --drop-empty-cells
- --extra-keys

Copy link
Member

Choose a reason for hiding this comment

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

It's good to add this arg for delete empty cell(s) in jupyter notebook, see e.g. https://github.com/ComPWA/gluex-nstar/blob/main/.pre-commit-config.yaml#L41C11-L41C31

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🖱️ DX Improvements to the Developer Experience
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Define developer environment
3 participants