change edit_api to reflect server #941
Conversation
There was a problem hiding this comment.
Do we want to allow everyone to edit the data? Is this temporary? I can't imagine this is healthy long term, someone will come along and be destructive.
Of course we do still need a way for other users to at least suggest changes to be approved by the original uploader or someone from openml (hopefully mostly the former). Are there plans for this?
Codecov Report
@@ Coverage Diff @@
## develop #941 +/- ##
===========================================
- Coverage 88.06% 87.77% -0.29%
===========================================
Files 37 37
Lines 4364 4427 +63
===========================================
+ Hits 3843 3886 +43
- Misses 521 541 +20
Continue to review full report at Codecov.
|
Reading this, I'm also a bit worried about this. |
It is temporary that everyone can edit non-critical fields.
|
Wouldn't it make sense to at least restrict this to registered users? |
yes only registered users can do this , otherwise the server will throw an error "Authentication failed". I checked this through a post request for edit and fork APIS but, I think this is true for all requests that are not "get". |
There was a problem hiding this comment.
Looks good to me. We might want to refer to the documentation on API key configuration rather than leaving the commented out code in (https://openml.github.io/openml-python/master/examples/20_basic/introduction_tutorial.html#sphx-glr-examples-20-basic-introduction-tutorial-py). Either way works for me. I'll leave the merge for @mfeurer since he had left feedback on the PR.
* Create first section: Creating Custom Flow * Add Section: Using the Flow It is incomplete as while trying to explain how to format the predictions, I realized a utility function is required. * Allow run description text to be custom Previously the description text that accompanies the prediction file was auto-generated with the assumption that the corresponding flow had an extension. To support custom flows (with no extension), this behavior had to be changed. The description can now be passed on initialization. The description describing it was auto generated from run_task is now correctly only added if the run was generated through run_flow_on_task. * Draft for Custom Flow tutorial * Add minimal docstring to OpenMLRun I am not for each field what the specifications are. * Process code review feedback In particular: - text changes - fetch true labels from the dataset instead * Use the format utility function in automatic runs To format the predictions. * Process @mfeurer feedback * Rename arguments of list_evaluations (#933) * list evals name change * list evals - update * adding config file to user guide (#931) * adding config file to user guide * finished requested changes * Edit api (#935) * version1 * minor fixes * tests * reformat code * check new version * remove get data * code format * review comments * fix duplicate * type annotate * example * tests for exceptions * fix pep8 * black format * Adding support for scikit-learn > 0.22 (#936) * Preliminary changes * Updating unit tests for sklearn 0.22 and above * Triggering sklearn tests + fixes * Refactoring to inspect.signature in extensions * Add flake8-print in pre-commit (#939) * Add flake8-print in pre-commit config * Replace print statements with logging * Fix edit api (#940) * fix edit api * Update subflow paragraph * Check the ClassificationTask has class label set * Test task is of supported type * Add tests for format_prediction * Adding Python 3.8 support (#916) * Adding Python 3.8 support * Fixing indentation * Execute test cases for 3.8 * Testing * Making install script fail * Process feedback Neeratyoy * Test Exception with Regex Also throw NotImplementedError instead of TypeError for unsupported task types. Added links in the example. * change edit_api to reflect server (#941) * change edit_api to reflect server * change test and example to reflect rest API changes * tutorial comments * Update datasets_tutorial.py * Create first section: Creating Custom Flow * Add Section: Using the Flow It is incomplete as while trying to explain how to format the predictions, I realized a utility function is required. * Allow run description text to be custom Previously the description text that accompanies the prediction file was auto-generated with the assumption that the corresponding flow had an extension. To support custom flows (with no extension), this behavior had to be changed. The description can now be passed on initialization. The description describing it was auto generated from run_task is now correctly only added if the run was generated through run_flow_on_task. * Draft for Custom Flow tutorial * Add minimal docstring to OpenMLRun I am not for each field what the specifications are. * Process code review feedback In particular: - text changes - fetch true labels from the dataset instead * Use the format utility function in automatic runs To format the predictions. * Process @mfeurer feedback * Update subflow paragraph * Check the ClassificationTask has class label set * Test task is of supported type * Add tests for format_prediction * Process feedback Neeratyoy * Test Exception with Regex Also throw NotImplementedError instead of TypeError for unsupported task types. Added links in the example. Co-authored-by: Bilgecelik <38037323+Bilgecelik@users.noreply.github.com> Co-authored-by: marcoslbueno <38478211+marcoslbueno@users.noreply.github.com> Co-authored-by: Sahithya Ravi <44670788+sahithyaravi1493@users.noreply.github.com> Co-authored-by: Neeratyoy Mallik <neeratyoy@gmail.com> Co-authored-by: zikun <33176974+zikun@users.noreply.github.com>
Reference Issue
#929
What does this PR implement/fix? Explain your changes.
The server has made changes to the edit_api. Refer openml/OpenML#1058.
Summary of the changes:
Edit API - Edit non-critical data fields for everyone. Edit critical fields only for the owner, provided there are no tasks. If the owner already created a task, we will just throw an error.
Fork API - Clone the row as such with change in dataset ID and uploader ID.
How should this PR be tested?
Any other comments?