UofS-Pulse-Binfo / analyzedphenotypes

Tripal/Drupal support for analyzed phenotypic data including data loaders, exporters, trait pages and summaries on germplasm pages.
GNU General Public License v2.0
0 stars 1 forks source link

Reorganize the API #28

Closed laceysanderson closed 5 years ago

laceysanderson commented 6 years ago

Currently the API consists of grouping functions with the first parameter being the action to take and the second containing options related to that action. While this helps to group an arguably large API, it also makes it quite hard to read since there are large switch statements and the API function being called can be cryptic. Furthermore, the relatedness of the actions being grouped is a bit of a stretch in some cases. For example, analyzedphenotypes_cvprop handles everything from selecting all cvterms for a cv, finding a cvterm based on keywords, attaching a photo to a trait, relating two cvterms, etc.

Instead, we could separate most current functions (with multiple options) into their own files with each action/property becoming it's own function. For example, analyzedphenotypes_cvprop() would be moved to analyzedphenotypes.cv.api.inc and the documented property options would become their own function (example below).

property new function Purpose
match_key ap_get_cvterm Match a record in chado.cvterm given a keyword.
match_cvid ap_get_cv Match a record in chado.cv given a cv_id.
terms_in_cv ap_get_cvterm_options Fetch all records in chado.cvterm given a cv_id.
get_cvterm ap_get_cvterm Fetch records in chado.cvterm, chado.dbxref and chado.cvterm_relationship (term properties) given a trait.
insert_cvterm ap_insert_cvterm Insert a record into chado.cvterm.
cvterm_save_photo ap_save_trait_photo Handle saving of photo to a trait/cvterm.
cvterm_get_photo ap_get_trait_photo Get photo information.
suggest_term ap_suggest_cvterm Suggest cvterm name with the cv name it is in.
term_cv ap_get_cvterm Given a string containing cvterm name and cv name (eg. Genus (taxonomic_rank)) in fields, return the cvterm id.
term_id ap_get_term_string Given a cvterm id number return a string value containing the cvterm name and cv name in cvterm name (cv name) format.
default ap_get_cv_options All records in chado.cv.

Note: match_key, get_cvterm, term_cv are combined into a single function: ap_get_cvterm() because the goal of all three is to retrieve a cvterm. Note: switch statements are prone to introduce bugs due to the need to break out of each case. Furthermore, they are not very readable when there are many lines of code per case.

Tripal API conventions:

Task

For all current API functions with the switch($property) approach, create new functions for each $property following the Tripal API Conventions. Where it makes sense, these new functions could be grouped in a separate file to keep the functions together.

To aid in reviewing, commit after converting each existing function. The next commit should then be updating where that function is called. Create the PR after the first commit pair and then update the PR as you go. This will hopefully allow me to review as you go and help to avoid repetitive fixes in the PR Review stage.

reynoldtan commented 6 years ago

SYSTEM VARIABLES API - Test function calls:

// All system variables. $m = ap_define_variablenames(); dpm($m);

// Fetch Genus variable set (cv, db and ontology). $k = ap_get_systemvariable( array('variablename' => 'Lens'), array('set' => 'cvdbon', 'suffix' => 'allsuffix') ); dpm($k);

// Fetch Genus specific variable. $a = ap_get_systemvariable( array('variablename' => 'Lens'), array('set' => 'cvdbon', 'suffix' => 'cv') ); dpm($a);

// Fetch cv term method. $t = ap_get_systemvariable( array('variablename' => 'Method'), array('set' => 'terms') ); dpm($t);

// Fetch module options. $o = ap_get_systemvariable( array('variablename' => 'allownew'), array('set' => 'options') ); dpm($o);

reynoldtan commented 6 years ago

Function names and array keys updated.

reynoldtan commented 6 years ago

GENUS API TEST FUNCTION CALLS

// All genus. $j = ap_get_genus(); dpm($j);

// Relate genus to project and fetch project genus. $project_id = 44; // Change to a different project id. ap_set_projectgenus( array('project_id' => $project_id, 'genus' => 'Felis') );

$g = ap_get_projectgenus( array('value' => $project_id) ); dpm($g);

// All active genus - with configured cv, db and ontology. $c = ap_get_activegenus(); dpm($c);

reynoldtan commented 6 years ago

CV API TEST FUNCTION CALLS

// Get cvterm matching a name keyword. $n = ap_get_cvterm( array('keyword' => 'trait', 'genus' => 'Lens') ); dpm($n);

// Get cvterm id of cvterm name (cv name). $g = ap_get_cvterm( array('cvtermcv' => 'Abiotic Stress traits (Lentil Crop Ontology)') ); dpm($g);

// Get cvterm matching a cv id. $c = ap_get_cvterm( array('cv_id' => 566) ); dpm($c);

// Get cvterm by name. $h = ap_get_cvterm( array('name' => 'Abiotic Stress traits', 'genus' => 'Lens') ); dpm($h);

// Get cvterm by cvterm id. Request to return additional information. $j = ap_get_cvterm( array('cvterm_id' => 5426, 'genus' => 'Lens'), array('resultset' => 'full') ); dpm($j);

// Get cvterm by cvterm id. Request to return in cvterm name (cv name). $y = ap_get_cvterm( array('cvterm_id' => 5426, 'genus' => 'Lens'), array('resultset' => 'cvtermcv') ); dpm($y);

// Get cvterm by cvterm id. Request to return just the id and name. $w = ap_get_cvterm( array('cvterm_id' => 5426, 'genus' => 'Lens'), array('resultset' => 'idname') ); dpm($w);

// Get cvterm photo information. $p = ap_download_cvtermphoto(6138); dpm($p); // ap_upload_cvtermphoto() was used to upload this photo.

// Manage autocomplete field to search cvterm. // Suggest $s1 = ap_autocomplete_cvtermfields( array('keyword' => 'Len') ); dpm($s1);

// cvterm_id convert to cvterm name (cv name) format. $s2 = ap_autocomplete_cvtermfields( array('cvtermid_cvtermcv' => 5426) ); dpm($s2);

// cvterm name (cv name) format convert to cvterm_id $s3 = ap_autocomplete_cvtermfields( array('cvtermcv_cvtermid' => 'Abiotic Stress traits (Lentil Crop Ontology)') ); dpm($s3);

laceysanderson commented 5 years ago

PR Merged! Great work @reynoldtan