Skip to content

Move to a conda only install of pymc (local dev workflow) #304

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

Merged
merged 11 commits into from
May 6, 2024
6 changes: 3 additions & 3 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -47,10 +47,10 @@ For more instructions see the [Pull request checklist](#pull-request-checklist)

Always use a feature branch. It's good practice to never routinely work on the `main` branch of any repository.

1. Create a new environment using Python >=3.8, for example 3.11
1. Create the environment from the `environment.yml` file.

```bash
conda create --name CausalPy python=3.11
mamba env create -f environment.yml
```

Activate the environment.
Expand All @@ -59,7 +59,7 @@ For more instructions see the [Pull request checklist](#pull-request-checklist)
conda activate CausalPy
```

Install the package (in editable mode) and its development dependencies:
Install the package (in editable mode) and its development dependencies. The `--no-deps` flag is used to avoid installing the dependencies of `CausalPy` as they are already installed when installing the development dependencies. This can end up interfering with the conda-only install of pymc.

```bash
pip install --no-deps -e .
Expand Down
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
.PHONY: init lint check_lint test

init:
python -m pip install -e .
python -m pip install -e . --no-deps

lint:
ruff check --fix .
Expand Down
26 changes: 9 additions & 17 deletions docs/source/notebooks/did_pymc.ipynb

Large diffs are not rendered by default.

61 changes: 31 additions & 30 deletions docs/source/notebooks/did_pymc_banks.ipynb

Large diffs are not rendered by default.

32 changes: 12 additions & 20 deletions docs/source/notebooks/geolift1.ipynb

Large diffs are not rendered by default.

38 changes: 15 additions & 23 deletions docs/source/notebooks/its_covid.ipynb

Large diffs are not rendered by default.

64 changes: 28 additions & 36 deletions docs/source/notebooks/its_pymc.ipynb

Large diffs are not rendered by default.

47 changes: 20 additions & 27 deletions docs/source/notebooks/rd_pymc.ipynb

Large diffs are not rendered by default.

79 changes: 51 additions & 28 deletions docs/source/notebooks/rd_pymc_drinking.ipynb

Large diffs are not rendered by default.

65 changes: 30 additions & 35 deletions docs/source/notebooks/rkink_pymc.ipynb

Large diffs are not rendered by default.

70 changes: 32 additions & 38 deletions docs/source/notebooks/sc_pymc.ipynb

Large diffs are not rendered by default.

291 changes: 146 additions & 145 deletions docs/source/notebooks/sc_pymc_brexit.ipynb

Large diffs are not rendered by default.

17 changes: 17 additions & 0 deletions environment.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
name: CausalPy
channels:
- conda-forge
dependencies:
- arviz>=0.14.0
- graphviz
- ipython!=8.7.0
- matplotlib>=3.5.3
- numpy<1.26.0
- pandas
- patsy
- pymc>=5.0.0
- scikit-learn>=1
- scipy
- seaborn>=0.11.2
- statsmodels
- xarray>=v2022.11.0
16 changes: 1 addition & 15 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -26,21 +26,7 @@ requires-python = ">=3.8"
#
# For an analysis of this field vs pip's requirements files see:
# https://packaging.python.org/discussions/install-requires-vs-requirements/
dependencies = [
"arviz>=0.14.0",
"graphviz",
"ipython!=8.7.0",
"matplotlib>=3.5.3",
"numpy<1.26.0",
"pandas",
"patsy",
"pymc>=5.0.0",
"scikit-learn>=1",
"scipy",
"seaborn>=0.11.2",
"statsmodels",
"xarray>=v2022.11.0",
]
dependencies = []
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think the problem comes because you are deleting these dependencies and you are pip installing the package in the CI

Copy link
Collaborator

Choose a reason for hiding this comment

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

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'm not that experienced with GitHub workflows. What am I looking for? Are you referring to the environment-file: in the with: block?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I mean concretely: https://github.com/pymc-devs/pymc-experimental/blob/main/.github/workflows/test.yml#L16-#L70 Here they are installing the conda env and installing the library. However, I see that they have a setup.py so this might not apply to this case 🙈

How I see it environment.yml is a specification of an environment but you still need to indicate de package dependencies and I think this is why the tests are failing unless you keep the dependencies = [ ... ] in the pyproject.toml. Maybe @maresb has a hint on how to make this cleaner?

Copy link
Contributor

Choose a reason for hiding this comment

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

Hey, I'm on vacation now, but we should discuss (and update) #281.

As for Conda vs pyproject dependencies, ideally we keep both. Pyproject dependencies define the python-only dependencies that you have. On the other hand, Conda takes care of both Python and non-Python dependencies.

I haven't read carefully any of the details, so I may be missing stuff, but the way I like to handle this is to:

  1. Create a Conda environment that satisfies all the dependencies, Python or otherwise
  2. pip install the project into the Conda environment with the --no-dependencies flag so that pip installs only the project
  3. Run pip check to ensure all of the project's Python dependencies are satisfied within the Conda environment. This will trip on the case when you declare a Python dependency in pyproject.toml that is missing from your environment.yaml file. I don't really know of any tests that go in the other direction, and I don't know that it would even make sense.

Note that this means that your primary source of truth for a project's dependencies should be the pyproject.toml file, not the environment.yaml file! I think this is how it should be. It enables masochistic "experts" to do an unsupported pip install if they are so inclined.

Regarding #281, conda-lock can parse the pyproject.toml for the requirements and generate a lockfile, so apart from specifying special requirements, we may be able to replace the environment.yml with a lockfile.


# List additional groups of dependencies here (e.g. development dependencies). Users
# will be able to install these using the "extras" syntax, for example:
Expand Down