Skip to content

Conversation

@tweddielin
Copy link
Contributor

Upgrade wrapt package to 1.13.3 to fix the issue #863.

tweddielin and others added 4 commits January 24, 2022 14:59
running into an error importing `numpy.core.multiarray` in the python 3.9 tests, so hoping this will resolve it (without breaking other things...)
Looks like numpy 1.22 loses python 3.7 support, so let's give the minor version bump a try instead 🤷
@shaycrk
Copy link
Contributor

shaycrk commented Jan 26, 2022

@tweddielin -- seems like good news/bad news... apparently bumping the numpy version up to 1.22.1 fixes the python 3.9 error we've been getting here but at the expense of losing python 3.7 support. That's not ideal since I think many of our current projects have been using python 3.7, but we certainly want to be able to work with newer postgres versions.

I guess the other potential direction here is to remove the wrapt dependency entirely and try to implement the getstate/setstate approach we discussed? Thoughts?

BTW, converting the PR to a draft for the time being since I've been playing with the python version support...

@shaycrk shaycrk marked this pull request as draft January 26, 2022 04:23
@shaycrk
Copy link
Contributor

shaycrk commented Jan 26, 2022

Ok -- here's another option: using environment markers to specify different numpy versions for different python versions. I don't think this is a great long-term solution, but maybe a reasonable bandaid to get things working until we're ready to let go of python 3.7 support?

@tweddielin
Copy link
Contributor Author

@shaycrk looks like all tests pass with matplotlib removal from conftest.py

@shaycrk
Copy link
Contributor

shaycrk commented Jan 26, 2022

Thanks @tweddielin -- makes sense. Should we try switching back to the single version of numpy in that case?

@shaycrk
Copy link
Contributor

shaycrk commented Jan 27, 2022

Hmmm... looks like with the uniform version of numpy, we still get the same error, even with removing the matplotlib import in conftest.py:

==================================== ERRORS ====================================
________________________ ERROR collecting test session _________________________
.tox/py3/lib/python3.9/site-packages/_pytest/config/__init__.py:495: in _importconftest
    return self._conftestpath2mod[key]
E   KeyError: PosixPath('/home/runner/work/triage/triage/src/tests/conftest.py')

During handling of the above exception, another exception occurred:
.tox/py3/lib/python3.9/site-packages/py/_path/local.py:704: in pyimport
    __import__(modname)
.tox/py3/lib/python3.9/site-packages/_pytest/assertion/rewrite.py:152: in exec_module
    exec(co, module.__dict__)
src/tests/conftest.py:4: in <module>
    from tests.utils import sample_config, populate_source_data
src/tests/utils.py:9: in <module>
    import matplotlib
.tox/py3/lib/python3.9/site-packages/matplotlib/__init__.py:207: in <module>
    _check_versions()
.tox/py3/lib/python3.9/site-packages/matplotlib/__init__.py:192: in _check_versions
    from . import ft2font
E   ImportError: numpy.core.multiarray failed to import

---------- coverage: platform linux, python 3.9.10-final-0 -----------

=========================== short test summary info ============================
ERROR  - ImportError: numpy.core.multiarray failed to import
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 1.04s ===============================
ERROR: InvocationError for command /home/runner/work/triage/triage/.tox/py3/bin/py.test --basetemp=/home/runner/work/triage/triage/.tox/py3/tmp -vvv --cov=triage (exited with code 2)
___________________________________ summary ____________________________________
ERROR:   py3: commands failed
Error: Process completed with exit code 1.

Looks like it's now running into a matplotlib import issue in test.utils now causing the same problem. Will go back to the two versions of numpy for the time being, but @tweddielin not sure if you want to keep digging on the root cause of the error here?

@tweddielin
Copy link
Contributor Author

@shaycrk looks like updating matplotlib to 3.3.4 works

@shaycrk
Copy link
Contributor

shaycrk commented Jan 27, 2022

Interesting -- thanks @tweddielin! Strange that it was causing that problem, but glad that works and can avoid the divergent numpy versions!

@shaycrk
Copy link
Contributor

shaycrk commented Jan 29, 2022

Just tested with a fresh ec2 instance and appears to work with both python 3.7 and 3.8 across postgres 11, 12, and 13. What's confusing is that I'm now getting the pickling error with triage 5.0.0 (wrapt 1.12.1) and postgres 11.12 (the same version as our main database), which wasn't happening before in this testing and we haven't seen with other projects...

Regardless, the wrapt update seems like it solves this problem, so we should probably go ahead and pull it in and tag a bugfix version. Changing back to a standard PR for review.

@shaycrk shaycrk marked this pull request as ready for review January 29, 2022 06:10
@shaycrk shaycrk requested a review from thcrock January 29, 2022 06:10
@thcrock
Copy link
Contributor

thcrock commented Feb 7, 2022

@shaycrk When you assigned it to me did you intend for me to merge it once I approve?

@shaycrk
Copy link
Contributor

shaycrk commented Feb 8, 2022

Thanks @thcrock! Either way is fine with me -- I'll go ahead and merge it now and tag a version for pypi.

@shaycrk shaycrk merged commit 3ca63dd into master Feb 8, 2022
@shaycrk shaycrk deleted the update-wrapt-version branch February 8, 2022 22:03
@shaycrk shaycrk mentioned this pull request Feb 8, 2022
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.

4 participants