Closed sappelhoff closed 3 years ago
sappelhoff
commented
3 years ago one more thing: Running the test and docs build (or one of the two) downloaded some data, which are not git-ignored, and these show up in the git diff. A potential contributor may be confused by that and/or accidentally commit that data. If you can think of a way on how to avoid this (docs, better .gitignore, ...), that'd be great I think.
turbach
commented
3 years ago @sappelhoff right, we don't want sample data committed, the downloads will be gitignored in the next patch release.
turbach
commented
3 years ago @sappelhoff thanks for the dev environment test drive. Your approach is generally sound and along the lines of the suggestions in the How to contribute docs. I don't think it's a serious issue tho to diagnose I'd probably want to look at your .condarc, an MRE that includes the conda activate ... commands and a conda list dump to see what version of what got installed from which channel into the environment that was active for the pytest and sphinx docs build. I'm adding a bug report .github/ISSUE_TEMPLATE to the next fitgrid patch release to help spell this out.
But. All of this is moot at the moment because ...
While I was out last week, mamba 0.15.0 dropped and changed the semantics of strict channel priority.
I expect this is a breaking change for the mamba installation procedures in the current fitgrid v0.5.1 docs and is definitely a breaking change for our CI workflow and I guess others b.c. of mamba-org/boa#165. Apparently fixes are in the works, when the dust settles I'll revise the fitgrid CI and docs accordingly and @ you to wrap up this issue.
sappelhoff
commented
3 years ago Alright, that's unfortunate. Anyhow, below is a bit more information. It's a great idea to add an issue template for this :+1:
As for JOSS, I am going to tick off this item then, as the situation seems to be quite in flux and this is something that's unlikely to be resolved in a matter of days. I'll still leave this issue open for you to resolve as "the dust settles", as you say :-)
Using conda with version `, I am running the following commands in succession from my (clean) condabase` environment:
conda create --name fg fitgrid -c kutaslab -c ejolly -c conda-forge --strict-channel-priority --yesconda activate fgconda install black pytest pytest-cov codecov -c conda-forge --yesconda install sphinx sphinx_rtd_theme sphinx-gallery pandoc -c conda-forge --yesThen I run (from up to date fitgrid main):
pytestmake -C docs/ htmlBoth commands complatin about lmertest not being installed (see my original post).
So I run:
conda install -c conda-forge r-lmertest --yesAnd that results in the other error reported above (some rpy2 thing).
Click "details" below to see output of conda list from the fg (fitgrid) environment.
BEFORE installing lmertest:
AFTER installing lmertest:
turbach
commented
3 years ago Thanks @sappelhoff, this is good info. I replicated the issue, you're fine but I got bitten by the not-quite interchangeability of conda and mamba 0.14.
Diagnosis
The BEFORE env shows that with -c kutaslab -c ejolly channel order and strict channel priority, the conda solver locks onto our legacy pymer4 0.6.0 conda package on -c kutaslab. That 0.6.0 version is our in-house conda skeleton snapshot of the pymer4 0.6.0 PyPI package back when pymer4 wasn't packaged for conda elsewhere. It doesn't have the r-* dependencies baked in (see ~/path/to/envs/fg/conda-meta/pymer4-0.6.0-py36_0.json) so they have to be installed manually. As you found with the AFTER env, that doesn't always go smoothly even with dependency resolution.
What we want nowadays for fitgrid installation are the more recent pymer4 0.7 conda packages on anaconda.org/ejolly/pymer4 where the R dependencies are specified in github.com/ejolly/conda/meta.yaml and where the conda packages are built, installed, and pytested in the CI so that mindless computers check the installation and execution as installed before we humans try to use it. If that sounds familiar, it should ejolly/pymer4#76.
Interestingly (if that's the right word), mamba 0.14 did the right thing for the wrong reason ... despite the strict channel priority it saw past the pymer4 0.6.0 on -c kutaslab and found the newer pymer4 0.7 on -c ejolly. Don't know why, hope to never learn.
A fix
So the question now is how to get the desired behavior using conda 4.10 or the new mamba 0.15 and its more conda-like (exactly conda-like?) treatment of strict channel priority. I think juggling the channel order is the simplest fix so that's what I included in the new fitgrid 0.5.2 docs:
conda create --name fg -c conda-forge -c ejolly -c kutaslab fitgrid --yesSo far in testing this install pattern seems to work for latest versions of fitgrid and the different combinations of (Python 3.7 vs. 3.8) x (conda vs. mamba) x (MKL vs OpenBLAS) x (strict vs. flexible channel priority) x (conda-forge vs. anaconda.org defaults channel priority) EXCEPT -c defaults -c conda-forge --strict-channel-priority but that didn't work before either.
For building dev environments as opposed to working environments, best practices are to use the -c kutaslab/label/pre-release channel in case there are updates to dependencies and pins in conda/meta.yaml on the dev branch that haven't yet made it into the stable release. There aren't at the moment but that can change at any time as packages churn on conda-forge.
conda create --name fg -c conda-forge -c ejolly -c kutaslab/label/pre-release fitgrid --yesIf you want to keep going with this and can live with Python 3.7 or 3.8, I think re-running w/ the new channel order would be the quickest way to get the development environment built. If you do and run into anything else by all means post the issue. Thanks, much appreciated.
sappelhoff
commented
3 years ago Thanks for the analysis @turbach. I am happy to report that with that channel order tweak both the tests and docs pass :slightly_smiling_face:
I'll leave this issue open for you to either close now, or when you made the adjustments to the installation docs.
turbach
commented
3 years ago @sappelhoff great thanks for the followup, glad it worked out. The updates are in the 0.5.2 docs for the JOSS release, should be good for now.
(related to https://github.com/openjournals/joss-reviews/issues/3293)
Thanks for all your changes @turbach. I have stumbled over a (probably small) thing:
I installed fitgrid according to https://kutaslab.github.io/fitgrid/installation.html#fitgrid-stable-release, using
condawith-c conda-forgeI then followed the CID script and installed test dependencies:
https://github.com/kutaslab/fitgrid/blob/5e917e6b149631006b63a654e28ff3a7900effbd/.github/workflows/fitgrid-cid.yml#L156
and docs dependencies:
https://github.com/kutaslab/fitgrid/blob/5e917e6b149631006b63a654e28ff3a7900effbd/.github/workflows/fitgrid-cid.yml#L169
When running
pytestandmake -C docs htmlI frequently got the following error:rpy2.robjects.packages.PackageNotInstalledError: The R package "lmerTest" is not installed.even after installing it via
conda install -c conda-forge r-lmertest, I got a different error for the tests and for building the docs:AttributeError: module 'rpy2.robjects.pandas2ri' has no attribute 'ri2py'Is there (1) a real issue here, or (2) did I do something wrong?. In the case of (2): It'd be good to improve the docs for contributors on which additional packages to install to properly run the tests and build the docs.