Skip to content

Conversation

@mfeurer
Copy link
Collaborator

@mfeurer mfeurer commented Oct 29, 2019

Closes #858 by using the live instead of the test server.

@PGijsbers
Copy link
Collaborator

Looks like tests fail due to server issues, otherwise the change looks good to me.

@mfeurer
Copy link
Collaborator Author

mfeurer commented Oct 29, 2019

Yes, trying to fix them together with @janvanrijn (this PR is part of it).

@mfeurer mfeurer merged commit 4a13100 into develop Oct 29, 2019
@mfeurer mfeurer deleted the fix858 branch October 29, 2019 09:46
@janvanrijn
Copy link
Member

Looks like tests fail due to server issues, otherwise the change looks good to me.

I disagree.

The unit tests seem to assert that there are 2 public versions of the iris dataset on the test server, which is an incorrect assumption, especially after we reset the testserver.

@mfeurer
Copy link
Collaborator Author

mfeurer commented Oct 29, 2019

The unit tests seem to assert that there are 2 public versions of the iris dataset on the test server

This PR changes this. The unit test now assumes that there are 2 public versions of the iris dataset on the live server.

@janvanrijn
Copy link
Member

That seems perfectly reasonable to me. This solves issue #858.

@PGijsbers
Copy link
Collaborator

I meant the CI results on this PR came back negative, but those were due to server issues. This was unrelated to the change made for this PR, which was not a server issue.

@janvanrijn
Copy link
Member

can you point out specifically which failing unit tests you refer to?

Some might fail due to recent reset of test server.

@PGijsbers
Copy link
Collaborator

Errors like this. I don't think they are persistent bugs, and might well have to do with the recent reset - I'm not saying the server is broken. But for the purpose of an openml-python PR I leave a comment indicating them as 'server issue' for later reference, indicating that we do see CI failures but don't expect them to be caused by the changes to openml-python.

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.

test__name_to_id_with_multiple_active_error assumes there are multiple active public iris datasets on test server

4 participants