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

WIP:add pymer4 v0.8.0 #22346

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open

WIP:add pymer4 v0.8.0 #22346

wants to merge 3 commits into from

Conversation

leej3
Copy link
Contributor

@leej3 leej3 commented Mar 20, 2023

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pymer4) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/pymer4:

  • Recipes should usually depend on matplotlib-base as opposed to matplotlib so that runtime environments do not require large packages like qt.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pymer4) and found it was in an excellent condition.

@leej3
Copy link
Contributor Author

leej3 commented Mar 27, 2023

On windows the CI is failing with:

Traceback (most recent call last):

File "C:\bld\pymer4_1679402624145\test_tmp\run_test.py", line 2, in <module>

import pymer4

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\pymer4\__init__.py", line 3, in <module>

from .models import Lmer, Lm, Lm2

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\pymer4\models\__init__.py", line 5, in <module>

from .Lmer import Lmer

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\pymer4\models\Lmer.py", line 10, in <module>

from rpy2.robjects.packages import importr

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\rpy2\robjects\__init__.py", line 19, in <module>

import rpy2.robjects.functions

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\rpy2\robjects\functions.py", line 12, in <module>

from rpy2.robjects import help

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\rpy2\robjects\help.py", line 45, in <module>

quiet_require('tools')

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\rpy2\robjects\help.py", line 41, in quiet_require

ok = _eval(expr)

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\rpy2\rinterface_lib\conversion.py", line 45, in _

cdata = function(*args, **kwargs)

File "C:\bld\pymer4_1679402624145\_test_env\lib\site-packages\rpy2\rinterface.py", line 817, in __call__

raise embedded.RRuntimeError(_rinterface._geterrmessage())

rpy2.rinterface_lib.embedded.RRuntimeError: Error in gettext(fmt, domain = domain, trim = trim) :

3 arguments passed to .Internal(gettext) which requires 2



Error in .makeMessage(..., domain = domain) :

3 arguments passed to .Internal(gettext) which requires 2

Error in gettext(fmt, domain = domain, trim = trim) :

3 arguments passed to .Internal(gettext) which requires 2

Tests failed for pymer4-0.8.0-py38haa244fe_0.conda - moving package to C:\bld\broken

WARNING:conda_build.build:Tests failed for pymer4-0.8.0-py38haa244fe_0.conda - moving package to C:\bld\broken

TESTS FAILED: pymer4-0.8.0-py38haa244fe_0.conda

I found a few issues (e.g. here ) with this sort of thing. One description of a fix indicates it can be caused by an unset R_HOME, or the R dlls dir was not added to python’s allowed dll directories. I believe r-base sets R_HOME in the activation script and that the issue is caused by the latter.

I can’t spot how bioconda get’s around this and I can’t find conda-forge packages that make use of rpy2. Based on the output of rpy2.situation it would seem that the package generally correctly detects the location of the shared libraries. The fix suggested of pre-pending this to LD_LIBRARY_PATH would be incompatible with conda. I wonder could rpy2 source be patched to include this fix? Or perhaps a fix upstreamed… @conda-forge/help-r let me know what you think.

@leej3 leej3 changed the title add pymer4 v0.8.0 WIP:add pymer4 v0.8.0 Apr 5, 2023
@leej3
Copy link
Contributor Author

leej3 commented Apr 5, 2023

I spotted that this doesn't pull in r-base let alone r-lme4. I will come back to this.

@leej3
Copy link
Contributor Author

leej3 commented Apr 21, 2023

@ejolly, I wonder could you take a look at this. I won't have much more time to finish it off. It's almost over the line. The r dependencies need to be added (perhaps just r-lme4). Also, the issue on windows needs to be addressed... I've proposed a modification that could perhaps be made to the pymer4 source code.

I'm happy to discuss if you want some guidance in the process. Once it is released on conda-forge the infrastructure should release conda packages for each new pymer4 version with relatively little maintenance.

@ejolly
Copy link

ejolly commented May 2, 2023

Thanks for getting this started @leej3 ! I won't have time to look at this for a few weeks unfortunately (travel and work), but I'll try to circle back when I can!

@ejolly
Copy link

ejolly commented Sep 8, 2023

Hey @leej3 sorry for circling back to this so late. I'm not familiar with the conda-forge packaging process and have rather basic knowledge of conda packaging altogether (mostly trial and error). But if it's helpful, I've had luck with this yaml config on my repo's GA runners, which upload to my own channel. I'm not sure how different the config needs to be for conda-forge

The catch is I've never been able to get a working conda build for Windows, neither using Windows GA runners or trying on a local machine when I had access to one. My workaround has been building for linux and converting it after the fact using conda convert like here. This seems to provide a working install for most users, but beyond that I've more-or-less given up on "officially" supporting Windows for pymer4.

@leej3
Copy link
Contributor Author

leej3 commented Sep 8, 2023

Thanks @ejolly. I used your recipe to start this off and made the required changes to yield the current state.

I think ideally the remaining steps to finish this would be:

  1. add any r dependencies, I wasn't certain of them, perhaps you could do this? They should be explicitly listed.
  2. possibly fix the issue windows... this doesn't need to happen. If it is too much effort recipes are accepted without this support. With that said, my understanding is that the issue here is not with pymer4 itself but with rpy2: we could try to add a fix there...
  3. add some tests. These should be very basic and run very quickly. They would be a quick sanity check that the various dependencies across different languages have been correctly specified.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pymer4) and found it was in an excellent condition.

I do have some suggestions for making it better though...

For recipes/pymer4:

  • Recipes should usually depend on matplotlib-base as opposed to matplotlib so that runtime environments do not require large packages like qt.

@ejolly
Copy link

ejolly commented Sep 15, 2023

@leej3 Alrighty I modified the yaml file to add reqs and tests and also bumped the version to the latest working release (0.8.1). I don't have a good mental model for how the conda-forge release process works though. Is the idea that the runner will watch any pypi version updates and automatically build a new version for conda forge?

@saraedum
Copy link
Member

saraedum commented Jul 3, 2024

@leej3 Alrighty I modified the yaml file to add reqs and tests and also bumped the version to the latest working release (0.8.1). I don't have a good mental model for how the conda-forge release process works though. Is the idea that the runner will watch any pypi version updates and automatically build a new version for conda forge?

Yes, essentially. When a new version shows up on PyPI, a PR is created that updates things.

@saraedum
Copy link
Member

saraedum commented Jul 3, 2024

I believe that this doesn't require review yet and isn't ready to be merged, so I am removing the label.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

3 participants