Closed jpreszler closed 1 year ago
Not sure why the doctests are failing - they pass when I run then locally. Sometimes remote tests randomly fail and then pass when they are re-run, but that hasn't worked here (yet).
@drbenvincent The failure is coming from numpy when conftest is getting initialized, looks like it's the same issue on your website change pr. A new version was just released and the may have changed something. I've pinned version 1.24.4 (the most recent that github has before 1.26.0, I had 1.25.2 locally) and the tests are starting to run now.
Merging #240 (e2511f9) into main (234a0cd) will not change coverage. The diff coverage is
n/a
.
@@ Coverage Diff @@
## main #240 +/- ##
=======================================
Coverage 73.89% 73.89%
=======================================
Files 19 19
Lines 1149 1149
=======================================
Hits 849 849
Misses 300 300
Files Changed | Coverage Δ | |
---|---|---|
causalpy/pymc_models.py | 100.00% <ø> (ø) |
The issue is actually coming from https://github.com/pymc-devs/pytensor/blob/main/pytensor/link/c/cmodule.py#L2720 in pytensor. Since CausalyPy and PyTensor don't have upper limits on the numpy version, this line throws an exception since v1.26.0 (released on Sept. 16th) no longer has the get_info()
method.
Cool. I see you've dropped an issue in the pytensor repo https://github.com/pymc-devs/pytensor/issues/441
What do you think the best way forward is? Something like this?
numpy<1.26.0
like you've done and create an issue to resolve this in the future. I think for anything that requires pytensor, pinning numpy<1.26.0 is really the only solution for now.
On the pytensor side, I suspect there's been a better way to access the blas options for some time and they just need to update the code and likely have a higher minimum numpy version, but I don't know enough about this to be sure. If this is the case, a new pytensor version with that fix would than provide a choice for downstream packages and moving to the new pytensor version is probably the best longer term route.
This resolves issue #238