Added function to update the dataset status #529
Conversation
janvanrijn
changed the title
| def status_update(data_id, status): | ||
| """ | ||
| Updates the status of a dataset to either 'active' or 'deactivated'. Please | ||
| see the OpenML API documentation for a description of the status and all |
There was a problem hiding this comment.
Could you please add a link to https://docs.openml.org/#dataset-status ?
|
|
||
| openml.datasets.status_update(did, 'active') | ||
| openml.datasets.status_update(did, 'deactivated') | ||
| openml.datasets.status_update(did, 'active') |
There was a problem hiding this comment.
These calls only check if the function succeeds without crashing, but not whether it actually changes something on the server. Could you please check that something actually changed on the server.
Codecov Report
@@ Coverage Diff @@
## develop #529 +/- ##
===========================================
+ Coverage 89.91% 89.91% +<.01%
===========================================
Files 32 32
Lines 2915 2926 +11
===========================================
+ Hits 2621 2631 +10
- Misses 294 295 +1
Continue to review full report at Codecov.
|
mfeurer
left a comment
mfeurer
left a comment
There was a problem hiding this comment.
Looks good, but please have a look at my two comments before merging.
| import mock | ||
|
|
||
| import random | ||
| import six |
There was a problem hiding this comment.
Are you sure this is a good idea?
There was a problem hiding this comment.
Its only test server, i also put it in the java repo :\ don't know any other solution so it should be fine
|
|
||
| # admin key for test server (only adminds can activate datasets. | ||
| # all users can deactivate their own datasets) | ||
| openml.config.apikey = 'd488d8afd93b32331cf6ea9d7003d4c3' |
There was a problem hiding this comment.
I think this should not be in this PR but rather in the ones about uploading datasets. Unless there's a specific reason, could you please remove it before merging?
There was a problem hiding this comment.
I thought this came up through a merge conflict, thats why i put it in.. Will remove
There was a problem hiding this comment.
Correction, this is not part of this PR. If you look at develop branch, it is already in there. Shall I proceed with merging?
There was a problem hiding this comment.
Yes, please go ahead then.
status update fn #528