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

pymc-labs / CausalPy

A Python package for causal inference in quasi-experimental settings

https://causalpy.readthedocs.io

Apache License 2.0

829 stars 53 forks source link

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

Closed drbenvincent closed 1 month ago

drbenvincent commented 4 months ago

Closes #297
~Closes #287~ (Addressed by #309)
Core dependencies are ~~taken out of pyproject.toml and moved into environment.yml~~ duplicated in environment.yml
Instructions for setting up dev environment are updated in CONTRIBUTING.md
I re-ran the notebooks which show WARNING (pytensor.tensor.blas): Using NumPy C-API based implementation for BLAS functions. which is indicative of pymc being pip installed rather than conda installed.

review-notebook-app[bot] commented 4 months ago

Check out this pull request on

See visual diffs & provide feedback on Jupyter Notebooks.

Powered by ReviewNB

drbenvincent commented 4 months ago

Looks like the ci workflow is failing. I'm not strong on GitHub actions (see ci.yml I believe), so would appreciate any pointers. But it looks like the changes have broken the remote environment setup, which I suspected would happen :)

drbenvincent commented 2 months ago

Just updated requirement to pymc>=5.14.0, in order to keep up with #325

drbenvincent commented 1 month ago

When building the environment (locally) using the methods in this PR (i.e. conda):

all tests pass
12 doctests fail

That suggests that it's not a pip/conda environment construction issue

We also get 12 failing doctests in the remove tests.

codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 77.27%. Comparing base (9169718) to head (2bf31e1).

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #304 +/- ## ======================================= Coverage 77.27% 77.27% ======================================= Files 21 21 Lines 1395 1395 ======================================= Hits 1078 1078 Misses 317 317 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

drbenvincent commented 1 month ago

Would be good to get a sanity check to ensure I won't break everything before we merge this :)

maresb commented 1 month ago

At a glance this looks sound, but this seems to be completely user-facing, meaning it doesn't seem like anything is being validated/tested in the CI.

maresb commented 1 month ago

(At worst you're breaking something minor in the development workflow.)

maresb commented 1 month ago

On the CI front, it would be nice to move forward on #281.

drbenvincent commented 1 month ago

Yes, this one is user-facing. Will try to progress the ci PR very soon.