Tripal is scientific tool that integrates Drupal with the GMOD Chado database schema (http://gmod.org/wiki/Chado_-_Getting_Started). Chado is community-developed and supported for storage of biological data, typically genomic and genetic but recently expanded for diversity and breeding data. Tripal has a Chado installer and data loaders for loading common data formats into Chado including controlled vocabularies and other popular scientific file formats. A bulk loader is also available for importing of data from tab delimited files. Tripal also creates Drupal nodes for content housed in the Chado database and provides a highly customizable set of Drupal template files for all data types. The 'Tripal theme' project (not included in this module) serves as a base theme and provides the templates used by Tripal. Tripal integrates with Views and uses the Views API to expose all of the Chado tables. In summary, a scientific community can use Tripal and Drupal to easily construct an interactive website to facilitate data mining and discovery for their respective community.

Tripal is an international collaborative effort. We have several developers scattered over the US and Canada. And a worldwide user base. It has been growing for several years and already has a wealth of resources including a demo website, step-by-step tutorials, an active mailing list, a pre-loaded Virtual Machine, a documented API, and a dedicated home page (http://tripal.sourceforge.net). Originally, Tripal was housed at Sourceforge, but we desire to move this project to its proper place on the Drupal repository.

Here is the sandbox page: http://drupal.org/sandbox/spficklin/1337878
To clone the project: git clone --recursive --branch 6.x-1.x http://git.drupal.org/sandbox/spficklin/1337878.git tripal
This code is currently compatible with Drupal 6 (we have begun the process of migrating to Drupal 7).

Comments

FranciscoLuz’s picture

FranciscoLuz commented

Hi,

Here are some observations before any further do:

  1. You should follow the git branch naming convention http://drupal.org/node/1015226
  2. What differs your module from this module http://drupal.org/project/gmod_dbsf?
  3. Would you consider working with http://drupal.org/project/gmod_dbsf instead? Why?
  4. After you get the branch name fixed, you should run an automatic review at http://ventral.org/pareview.
    Ventral will check if your code complies with Drupal's code standards. I could not run it at this time because of the branch naming issue.
FranciscoLuz’s picture

FranciscoLuz commented
Status: Needs review » Needs work
spficklin’s picture

spficklin commented

Hi FranciscoLuz. Thank you for taking the time to review Tripal, and for your suggestions! Your time is much appreciated.

We have fixed the branch naming and updated the Issue text above to use the branch name 6.x-1.x. I have run the branch through the Ventral and it looks good. We only have one Issue reported by Coder and that is when it confuses an SQL statement that calls a stored procedure. It tells us we need to wrap the table in brackets {}... but it's a select from a stored procedure.

The objectives for Tripal and GMOD_DBSF are different. Tripal allows a research community to build a "model organism" database for their species of interest. The goal is to simplify web development so that a small lab can setup a web site with the functionality of large genome database but with fewer resources. GMOD_DBSF provides a platform, through Drupal, for bioinformatic analysis. Tripal does not provide any mechanism for performing data analysis. Simply data loading, storage and mining.

Both Tripal and GMOD_DBSF use Chado--it is a community standard database schema used by many tools. There is definite room for collaboration between our two projects so that data analyses performed by GMOD_DBSF could be dumped into the Chado schema used by Tripal, but the two projects can be installed together on the same Drupal instance and work side by side without interfering with the other. In fact one of our Tripal-based websites does just that.

laceysanderson’s picture

laceysanderson commented

I would like to expand a bit on why I think GMOD DBSF and Tripal should remain separate modules. There are many use cases where Tripal may be used without the want or need for GMOD DBSF functionality and vise versa. Furthermore, it is my belief that this is the case for the main user groups for each of the mentioned modules.

For example, one of the main uses for Tripal is as the main website for dissemination of biological data for a given organism. For example, http://medicago.org/, http://www.rosaceae.org/, http://flybase.org/ and many others. These websites are geared towards distributing the data from research groups around the world in one central place. This is something that Tripal excels at by providing generic and custom loaders and integrating with views and views_data_export to make display and download of data subsets easy. These websites receive the data after it has been analyzed and interpreted by the research group who generated it. As such they have no need for the GMOD DBSF bioinformatic analysis platform. Furthermore, bioinformatic analysis is often a very CPU intensive process which may be beyond their resources. In this case, bundling Tripal and GMOD DBSF together might actually deter people from using Tripal.

This goes both ways, insofar as a large user group for GMOD DBSF, individual researchers or small research laboratories, do not need the data dissemination tools that Tripal provides since they are already provided by the user group in the previous example. They are creating a website solely with the goal of analyzing and interpreting their data -this is what GMOD DBSF is geared towards providing. Making Tripal part of GMOD DBSF would cause additional installation and setup/configuration for this user group for functionality that is not needed.

By keeping the modules separate, they continue to serve the user groups they were designed for without additional overhead. Furthermore, use cases where both might be needed would simply install both modules on the same website.

laceysanderson’s picture

laceysanderson commented
Status: Needs work » Needs review

Changing this to "needs review" since all the points mentioned in #1678248-1: Tripal have been addressed.

krylov’s picture

krylov commented

Hi,-
1. your git link http://git.drupal.org/sandbox/spficklin/1337878.git lands at 404
2. move
14 require('tripal_analysis.api.inc');
15 require('tripal_analysis_privacy.inc');
into the functions where they are used. Replace with require_once
3. in lines 348-387 consider using the ternary operator for shorter code
4. lines 917-948 consider using .tpl to format your output
5. lines 1020-1124 remove the unused code from the module

spficklin’s picture

spficklin commented

Hi krylov,

Thanks for reviewing!

I don't know why the GIT link returns 404 error. I cut and pasted it from the project page. If I run this command it downloads fine:

git clone --recursive --branch 6.x-1.x http://git.drupal.org/sandbox/spficklin/1337878.git tripal

For item #2: Is it standard practice to put require statements in the functions where they are needed? We can certainly do that for some of our include files, as they are only meant to keep the module file concise, but Tripal has an API that should be available to all other modules. Tripal is quite large and it would be cumbersome to go through all of the code to put requires in every function where they are needed. We can do so if this is a requirement, but if we're not breaking any serious rules can we leave it as is?

For item #3: Thank you for the suggestion. We'll discuss it in our group. If it is a requirement we'll make the change.

For Item #4: Thank your for noticing. This was code that was old and no longer needed. I have removed it.

For item #5: The unused code has been removed.

patrickd’s picture

patrickd commented

git links always land at 404 if you open it by browser...
use git clone

do NOT use require in functions - use module_load_include()

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

@patrickd: "require" or "require_once" is perfectly fine for including files of your own module. module_load_include() should only be used if you want to include something from a completely different module.

patrickd’s picture

patrickd commented

ok sorry :)

krylov’s picture

krylov commented

Re your survey: That's not a bad example of what should not happen during the review process:)

spficklin’s picture

spficklin commented
Priority: Normal » Critical

As per instructions here: http://drupal.org/node/539608 updating status to 'critical'.

We have been actively developing while waiting for full project approval. So, if you find errors that Coder brings up we can easily fix those.

spficklin’s picture

spficklin commented

Hey all, I hate to be a nag? Any chance we could get some further review?

Thanks

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented

We are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

grisendo’s picture

grisendo commented

You still have some errors to fix: http://ventral.org/pareview/httpgitdrupalorgsandboxspficklin1337878git

  • The "?>" PHP delimiter at the end of files is discouraged, see http://drupal.org/node/318#phptags
    • ./extensions/tripal_search_unigene/tripal_search_unigene.views.inc
  • Bad line endings were found, always use unix style terminators. See http://drupal.org/coding-standards#indenting
    • tripal_analysis/tripal_analysis_privacy.inc
    • tripal_bulk_loader/tripal_bulk_loader.admin.inc
  • Remove all old CVS $Id tags, they are not needed anymore.
  • PHP Parse error: syntax error, unexpected '=', expecting '(' in ./tripal_views/views/handlers/chado_views_handler_field_counter.inc on line 13
spficklin’s picture

spficklin commented

Thank you, Klausi, for your suggestion. We'll work on doing some reviews.

spficklin’s picture

spficklin commented

Hi Grisendo,

Thanks for taking the time to review the code. Unfortunately, our 7.x-1.x version is not the one we are trying to get approval for. We had tried to port the code to Drupal 7 but we had quite a few features & bug fixes to the 6.x-1.x version that we wanted to get that version approved first and then we'd start doing development work on 7.x.

Here is the check of the 6.x-1x version with ventral: http://ventral.org/pareview/httpgitdrupalorgsandboxspficklin1337878git-6...

Also, we have been doing bug fixes and including some code in the 6.x-1.x branch while we've been waiting for review. And we've inadvertently broken some style rules. I've fixed most of those just now and I have a few left to go. I'll post back when I'm finished fixing those and we'll stop doing updates so the code can be reviewed without problem. I'll let you know when it's ready again to be checked.

Thanks again,
Stephen

TeeHays’s picture

TeeHays commented

Hi, I started having a look through the project; but it's absolutely huge and I cannot devote so much time to this. However, I did come across a few things which might be worth commenting on. Take them or leave them; they're just suggestions. I only looked at these two files.

Thanks.

File : tripal_analysis.api.inc
Line : 106 : Suggest not using SELECT *; Specify exact columns you want returning. Extra columns could be added to the table and your query may not need to return them all.

File : tripal_analysis.module
Line : 161 : Suggest not using SELECT *; Specify exact columns you want returning. Extra columns could be added to the table and your query may not need to return them all.

Line 355 - 407 : Suggest adding in some validation checks for these textfields.
You can use #validate or #element_validate to point to a function which you can write to check suitable data has been entered.
http://drupal.org/node/207025

Line : 520 : Suggest not using SELECT *; Specify exact columns you want returning. Extra columns could be added and your query may not need to return them all.
Line : 537 : Suggest not using SELECT *; Specify exact columns you want returning. Extra columns could be added and your query may not need to return them all.

anwar_max’s picture

anwar_max
Primary language English
commented
Status: Needs review » Needs work

I think people forget to change status.

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Status: Needs work » Closed (won't fix)

Closing due to lack of activity. Feel free to reopen if you are still working on this application.

If you reopen this please keep in mind that we are currently quite busy with all the project applications and I can only review projects with a review bonus. Please help me reviewing and I'll take a look at your project right away :-)

Also, you should get your friends, colleagues or other community members involved to review this application. Let them go through the review checklist and post a comment that sets this issue to "needs work" (found some problems with the project) or "reviewed & tested by the community" (found no major flaws).

klausi’s picture

klausi
he/him||they/them
Primary language German
Location 🇦🇹 Vienna
commented
Issue summary: View changes

Changed the branch from 6.x-0.4-dev to 6.x-1.x and updated the text to reflect the change.