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

bumped versions of dependencies #978

Merged
merged 6 commits into from
Apr 6, 2022
Merged

bumped versions of dependencies #978

merged 6 commits into from
Apr 6, 2022

Conversation

dkeeney
Copy link

@dkeeney dkeeney commented Apr 5, 2022

First cut at bumping the versions of all dependencies

@dkeeney dkeeney self-assigned this Apr 5, 2022
@dkeeney dkeeney added the build label Apr 5, 2022
@dkeeney
Copy link
Author

dkeeney commented Apr 5, 2022

This PR addresses Issue #975
Do not merge!
This iteration of the PR has a problem on Windows in py unit tests at bindings/py/tests/regions/network_test.py line 378
Testing with Visual Studio 2022 and Python 3.9.6

@dkeeney
Copy link
Author

dkeeney commented Apr 5, 2022

Should we bump the versions on the Python dependencies as well?

@ctrl-z-9000-times
Copy link
Collaborator

I don't think we need to bump the python dependencies because IIRC "setup.py" will automatically install the latest version.
It should be checking pypi.org to get the latest versions of everything.

@dkeeney
Copy link
Author

dkeeney commented Apr 5, 2022

ok, that is why I asked. It's just that requirements.txt specifies a version. I don't know if that just specifies a minimum or locks it to that version.

@dkeeney
Copy link
Author

dkeeney commented Apr 5, 2022

This pickle problem is not going to be easy to find.
What I need is the ability to capture the C++ log (stdout) when it is ran from python. If I had that I could turn on some tracing and see how far it got before it crashed.

I thought we had something to do that but it has been so long ago and my 75 year-old brain is not recalling it.

@ctrl-z-9000-times
Copy link
Collaborator

Ok, I just checked it. Yes, our "requirements.txt" only specifies minimums.


The python test cases will display anything printed to std-out when they fail, its part of the test summary that's printed at the end.

The place where the error is generated, binascii, is part of the python std-lib and it appears to be a compiled C extension, so i think it's going to be very difficult to modify that code with tracing statements.

I assume that this bug is the result of updating the cereal library?
Maybe we could update to an older version or update everything except cereal, and see if the problem persists.

@dkeeney
Copy link
Author

dkeeney commented Apr 5, 2022

backing out cereal had no effect.
I put back cereal to latest and backed out pybind11.
It fixed the serialization problem but caused a different error in anomaly_likelihood_test.py line129 and 81
Thinking it might be a timing problem I ran it a second time and it was the same result.

I will push this and you can take a look at it.
In the mean time I will try some other combinations.

@ctrl-z-9000-times
Copy link
Collaborator

error in anomaly_likelihood_test.py line129 and 81

That's my fault. I think I introduced those errors when I backed out a change to the anomaly likelihood code.
I'm not entirely sure what to do about those test failures.
The comments describing those 2 testcases do not exactly match what the test actually does.
I think I can fix those tests by setting the anomaly detection threshold to a different value for each?

Copy link
Collaborator

@ctrl-z-9000-times ctrl-z-9000-times left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I retested this (linux, release mode) and it all passes except for the 2 failing anomaly likelihood tests. This all looks good to me!

@dkeeney dkeeney merged commit 2444639 into master Apr 6, 2022
@dkeeney dkeeney deleted the dep_upd branch April 6, 2022 19:21
@dkeeney
Copy link
Author

dkeeney commented Apr 6, 2022

Ok, the changes to the dependency versions is now in master.
Thanks @ctrl-z-9000-times and finnoshea for your help on this one.

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

Successfully merging this pull request may close these issues.

2 participants