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

Fix pytest #37

Merged
merged 10 commits into from
Oct 30, 2020
Merged

Fix pytest #37

merged 10 commits into from
Oct 30, 2020

Conversation

henrikef
Copy link
Contributor

Fix errors about "calling fixtures directly" in pytest. Tests pass for me locally now (except for one known failure in test_geminga_paper. Would be happy about feedback, I'm not an expert on pytest.

@henrikef
Copy link
Contributor Author

Ugh... Tests still fail.

For python 2, tons of ValueError: cannot set WRITEABLE flag to True of this array. ( https://travis-ci.org/github/threeML/hawc_hal/jobs/738193403#L1850 )

For python 3, pip install hawc_hal fails, see #38

🙈

@henrikef
Copy link
Contributor Author

Ok. The good news is that the install seems to work & tests pass for python 3.7 (at least for macos).

For python 2.7, there are still issues: In the Mac environment, the conda environment could not be resolved. In the linux one, conda installed a really old version of astromodels/threeML (and some other packages) for some reason, leading to compatibility issues down the line. I'm looking into it, but not sure what's going on.

Do we need to keep python 2.7 support?

@ndilalla
Copy link
Contributor

ndilalla commented Oct 29, 2020

Ok. The good news is that the install seems to work & tests pass for python 3.7 (at least for macos).

For python 2.7, there are still issues: In the Mac environment, the conda environment could not be resolved. In the linux one, conda installed a really old version of astromodels/threeML (and some other packages) for some reason, leading to compatibility issues down the line. I'm looking into it, but not sure what's going on.

Do we need to keep python 2.7 support?

Hi @henrikef, I think the reason is that the script is trying to install root5 but this is no longer supported since version 2.0.0. You can also "help" conda to solve the environment by requiring to install "threeml>=2" and "astromodels>=2". Let me know if it helps.

@henrikef
Copy link
Contributor Author

Thanks! I've been trying offline to get HAL running in a python 2.7 environment, but no luck so far. I'm having problems installing root_numpy and there are some conflicts with the chainconsumer package, I'm not even sure where that dependency comes from. I'll try again and make sure not to use root5.

@ndilalla
Copy link
Contributor

Mmh, interesting. Let me know if you need help with it.

@henrikef
Copy link
Contributor Author

Hm. I tried with conda create -n test-environment -c conda-forge -c threeml python=2.7 "astromodels>=2" "threeml>=2" healpy root and then pip install --no-binary :all: root_numpy, and I get a bunch of compiler errors (error: ISO C++17 does not allow 'register' storage class specifier).

Any ideas? (This is on macOS 10.15.7).

I attach the full output of both the conda install and the (failed) pip install below.

conda.log
pip.log

@ndilalla
Copy link
Contributor

Let me try on my laptop.

@henrikef
Copy link
Contributor Author

I will go ahead and merge this one for now, as we were not able to get the python2 tests to pass offline either, but python 3 seems fine.

@henrikef henrikef merged commit 379f0d1 into threeML:master Oct 30, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants