-
Notifications
You must be signed in to change notification settings - Fork 18
0.20rc1; fix not accessing correct installed sklearn path in AppVeyor #7
Conversation
Comparing the logs from the latest working appveyor build and this, I think I'm barking up the wrong tree, but might continue debugging in this PR. |
well the change was using pytest not nosetest (or at least that was the intention). But I know very little of what was going on here in the first place... |
Well I can see that the error seems to occur because it's reading from the wrong sklearn path. But I can't entirely understand why. So I'm trying just removing the checkout |
It's working! But we do have test failures. |
Several failures, it seems on the
run, including so far:
:( |
yay 32bit! |
(my plan is to be somewhat online at least until the RC is out btw) |
Failures are all doctest failures (ping @adrinjalali??): For example:
Apart from these platform-dependent things, we also get a doctest failure in MeanShift which may be unstable?? I can only guess that this was always unstable across platforms and we've only seen this because we now have a doctest. Not sure how to debug this.
|
Feel free to merge this so that we can then tweak on top of it? |
doctests are supposed to be skipped on 32bit. they are in our makefile. |
(for example because of the dtype thing). |
Well, then let's try running the makefile here. |
Actually, no, the makefile only skips tests in |
On Py=2.7, arch=64, we also have doctest failures like:
and
and
|
and conftest.py now skips doctests where numpy < 1.14, but doesn't make any other requirements. |
But I'm presuming these tests weren't checked on nose. I'm not sure how/when to disable doctests nicely in pytest, unless we modify conftest. (Why hasn't this been an issue in the scikit-learn repo appveyor?) |
It's funny. Even on 64-bit runs, we're getting test failures that imply a 32-bit environment. Things like:
Is it possible that even on 64-bit we are using a 32-bit version?? I can't see how that might be happening. |
@jnothman I'm not sure if there's a way to fix any of these (in a reasonable way). Basically, in a 64 bit system, the dtype is not printed if it's int64, and is printed if it's int32, and vice versa. The only fix I can think of, is to explicitly set the int type throughout the algorithms based on the architecture it runs on. But I don't think that's what we're going to do here. And it's the same with the other failing doctest examples. In Python3 all strings are unicode by default, and when Python2 sees unicode it puts the The For the other ones, I'm happy to help, but I'm not sure if/how we want to approach the issue. I've got the 32bit VM running, so I can do the tests easier now. |
Hm maybe @ogrisel remembers what exactly we're running usually? And yes, these errors on 64bit looks suspicious. I think we might be skipping 32bit and py27 but I'm not sure about the latter. |
we're running py27 doctests on travis from what I can see... but I should really start packing now ;) |
wait we're discarding the setup.cfg here, right? that might be the difference? |
It would make sense to discard
There is also a Docker setup in https://github.com/rth/scikit-learn-dev-docker/blob/master/debian/apt/Dockerfile that could be used with the https://hub.docker.com/r/i386/debian/ image to get a 32 bit environment without having to setup a VM.. |
This is using master from one month ago. If I understood the discussion right, it doesn't work with latest master? It would have been nice to have the latest one, otherwise we won't know when issues (reported in scikit-learn/scikit-learn#11878) are fixed.. |
Okay, I'm trying disabling setup.cfg to avoid doctests, and have updated scikit-learn to master. |
It looks like we have test failures on 32-bit architectures in both Travis and AppVeyor.
and sklearn/cluster/tests/test_optics.py:test_reach_dists seem to be the only culprits, at a glance. |
Proposed a fix for |
Is it? I think it's using master from three days ago? |
Looks like on travis only the Optics thing is failing. So we just change the precision for that? |
Updating master should hopefully make all the CI green. |
Now we get failures on Mac (all builds) for |
Sometimes testing rigorously shoots us in the foot? |
So on the first Mac run there we have we have with
and with
But on the second:
Note these are much closer, though they still fail. The third run:
On my macbook the test passes with:
In a 32-bit VM where I'd been testing OPTICS issues, I get:
Increasing max_iter changes the result. But the result is extremely unstable across platforms! I don't know what to do here... yet. |
A second run of the same CI gives different results. So the fit is indeed non-deterministic. |
Let's skip it and investigate later. |
More like discovering new bugs :) Though bugs never end unfortunately, and we should release regardless.. |
This should be ready for merge to produce wheels for the RC. |
Thanks, sorry for the lack of reply. Let me merge this and give you the rights to this repo so that you are not stuck again in the future. |
Thanks. Though I think I still can't upload the wheels to pypi. Also, appveyor looks like it is failing on master |
Try to fix appveyor testing