Bump scipy and matplotlib dependencies #757
Conversation
| @@ -219,7 +219,8 @@ def plot_style(fig, ax, # pylint: disable=too-many-arguments, too-many-locals | |||
| ax.xaxis.set_visible(False) | |||
| ax.yaxis.set_visible(False) | |||
|
|
|||
| ax.set_aspect(aspect_ratio) | |||
| if ax.name != '3d': | |||
There was a problem hiding this comment.
Without the if, matplotlib says:
NotImplementedError: It is not currently possible to manually set the aspect on 3D axes
whose recent version somehow break some tests And fix a line of code that does not work with newest matplotlib
neurom/tests/test_stats.py
Outdated
| @@ -166,11 +167,17 @@ def test_compare_two(): | |||
|
|
|||
| results2 = st.compare_two(data, data_close, test=st.StatTests.ks) | |||
| nt.assert_almost_equal(results2.dist, 0.5) | |||
| nt.assert_almost_equal(results2.pvalue, 0.5344157, places=5) | |||
| if sys.version_info[0] == 2: | |||
There was a problem hiding this comment.
If this is not scary, I don't know what is
There was a problem hiding this comment.
something is wrong, it's too much of a divergence; I'd be surprised if it is on scipy's side, so I think we should find the root cause before implementing a version test
There was a problem hiding this comment.
Given that scipy is the only thing that changed wrt previous commit I think it is on scipy's side. But yes, I'll investigate more.
There was a problem hiding this comment.
do the different python versions get different scipy versions?
There was a problem hiding this comment.
Yes, 1.3.0 has not been released for python2
There was a problem hiding this comment.
@lidakanari Do you have any idea how much the stats functionality is used?
It seems to mostly be a wrapper around scipy, and since scipy is augmenting their api, I wonder how much we should mirror it as opposed to letting the user directly call scipy.
There was a problem hiding this comment.
Personally I am running these tests independently, but I had a few questions from users taking advantage of our scipy wrappers. However, the validation suite uses scipy and ks_2samp so I think it is worth investigating what is the impact of this change.
There was a problem hiding this comment.
Maybe we could add a deprecation notice and remove it in a few months ?
There was a problem hiding this comment.
Is this used in the validations? If so, we need to replace it otherwise it will break the pipeline for circuit validations. Yes, the deprecation warning will definitely be useful.
There was a problem hiding this comment.
What is the validation pipeline ? Is it a repo ?
whose recent versions somehow break some tests
To be merged before #756