-
Notifications
You must be signed in to change notification settings - Fork 11
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
[JOSS Review] Tests #12
Comments
I will absolutely do that.
Whenever I run Could you rerun the tests from within the top level directory? This should then give you all the passing tests. Thanks again! |
When I run from the top-level directory, I get 12 failures:
|
Seems like all the remaining issues are pickle related. Maybe this hints at some sort of platform dependence in the pickling? |
Ah right so this seems like a python versioning issue after I noticed you are using python 3.9. (according to a similar issue here ). I am using 3.6 so I will need to do some testing for how to get around this change to allow all these versions |
You can specify the python version in the conda environment creation command to help with that. For example, I think you could just add However, I tried this and it led to package conflicts in the create env command, so I think you may need to specify the exact python 3.6 version you have. On the other hand, this might force your users to use python 3.6, so I'll leave it up to you to decide if you want to make the python version a specification of the environment or find some other workaround for the pickling. |
so I tried with python3.9 and I was only able to reproduce the issue when using macOS (i normally use linux and i still get all passes with 3.9). So I think the issue I linked above may be actually more useful than I originally thought as I am going to take an informed guess that you're using a mac? adding the suggested lines:
to the top of all_tests.py and the test do now pass although there is the issue of the reported crashing of the processes so that is definitely something to be wary about. |
Gotcha. Yeah I tried two things:
Both resulted in all tests passing, and in the second case I see the reported crashing of processes. Personally, I think the safest thing to do is to add python==3.7 to the requirements.txt file, but as long as your tests pass I'm not too worried about crashing processes that only occur when tests are run. So two options:
|
Awesome, glad its now working. I decided on going for option 2. |
Yup! This one looks good to me now |
This issue is part of the JOSS review going on in openjournals/joss-reviews#3635.
The
tests/
directory contains 122 tests, but lacks organization. Suggested changes are:The text was updated successfully, but these errors were encountered: