Skip to content

Added function to update the dataset status - #529

Merged
janvanrijn merged 5 commits into
developfrom
add_#528
Oct 10, 2018
Merged

Added function to update the dataset status#529
janvanrijn merged 5 commits into
developfrom
add_#528

Conversation

@janvanrijn

Copy link
Copy Markdown
Member

status update fn #528

@janvanrijnjanvanrijn changed the title openml server exceptionAdded function to update the dataset statusSep 18, 2018
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

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you please add a link to https://docs.openml.org/#dataset-status ?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Added


openml.datasets.status_update(did, 'active')
openml.datasets.status_update(did, 'deactivated')
openml.datasets.status_update(did, 'active')

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Added

@codecov-io

codecov-io commented Oct 10, 2018

Copy link
Copy Markdown

Codecov Report

Merging #529 into develop will increase coverage by <.01%.
The diff coverage is 90.9%.

Impacted file tree graph

@@ 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
Impacted FilesCoverage Δ
openml/datasets/__init__.py100% <ø> (ø)⬆️
openml/datasets/functions.py90.41% <90.9%> (+0.02%)⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update ba94609...b28fde8. Read the comment docs.

@mfeurermfeurer left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Looks good, but please have a look at my two comments before merging.

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Are you sure this is a good idea?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

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'

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

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?

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

I thought this came up through a merge conflict, thats why i put it in.. Will remove

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

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

Correction, this is not part of this PR. If you look at develop branch, it is already in there. Shall I proceed with merging?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Yes, please go ahead then.

@janvanrijn
janvanrijn merged commit bae83c6 into developOct 10, 2018
@janvanrijn
janvanrijn deleted the add_#528 branch October 10, 2018 15:18
Sign up for freeto 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.

3 participants

@janvanrijn@codecov-io@mfeurer