Conversation
janvanrijn
left a comment
There was a problem hiding this comment.
Code looking good @sahithyaravi1493, I could only find some small things that should be improved.
|
|
||
| private function data_edit() { | ||
| // Get columns to be update from post data | ||
| $data_id = $this->input->post('data_id'); |
There was a problem hiding this comment.
For consistency, it would be great if we would solve this with a XML (and new XSD) as well.
openml_OS/models/api/v1/Api_data.php
Outdated
|
|
||
| // If data id is not given | ||
| if( $data_id == false ) { | ||
| $this->returnError( 110, $this->version ); |
There was a problem hiding this comment.
For consistency, the API error codes are setup in such a way that we have a "range" of error codes per error function. It would be good to claim the range 1060 - 1069 for this function, and duplicate all error messages that are now reused.
openml_OS/models/api/v1/Api_data.php
Outdated
| // where data id | ||
| $where_data = 'where did='. $data_id; | ||
|
|
||
| $this->Dataset->query('update dataset set '. $update_total . $where_data); |
There was a problem hiding this comment.
please use Codeigniter official Query for this (it's $this->Dataset->update or something like that)
openml_OS/models/api/v1/Api_data.php
Outdated
| $where_data = 'where did='. $data_id; | ||
|
|
||
| $this->Dataset->query('update dataset set '. $update_total . $where_data); | ||
|
|
There was a problem hiding this comment.
please also 'catch' the return value of that function (boolean) and handle it if there is an error
openml_OS/models/api/v1/Api_data.php
Outdated
| $this->Dataset->query('update dataset set '. $update_total . $where_data); | ||
|
|
||
| // Return edited dataset, for user to verify changes | ||
| $this->xmlContents( 'data-get', $this->version, $dataset ); |
There was a problem hiding this comment.
For consistency (with upload/delete fns), I would suggest to create a 'data-edit' and to only show data id
janvanrijn
left a comment
There was a problem hiding this comment.
Thanks for making this PR! It looks great.
As discussed, this api can edit some meta-features of the dataset:
List of meta-features can be edited/ updated via data_edit API call,
These meta-features/ data will just need to call create_dataset api and create a new version: (python api)
After the data_edit API is ready, I can create a single python API to handle both of these cases.
Based on the arguments (None or otherwise). We can either create a new dataset or call the data_edit API.
How to test this API?
Create a post request:
Request URL: localhost/api/v1/json/data/edit
Request body: Specify the data_id and atleast one of the 7 fields to edit as an XML
example data.xml: