New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add cvalue_id to all prop tables #26

Closed
scottcain opened this Issue Dec 20, 2017 · 6 comments

Comments

Projects
None yet
4 participants
@scottcain
Copy link
Member

scottcain commented Dec 20, 2017

Adding a cvalue_id field to the biomaterialprop table, as is done for the natural diversity module. This allows the property values to also utilize a cvterm. The idea is to produce something similar to what was done with several natural diversity modules where values could be either be free text or from a controlled vocabulary.

See the related document outlining the suggestions for the 1.4 Chado release: https://docs.google.com/document/d/1t_Jb4XxUPtgGRSKtznsqvjTjmR4vpFHPQROMQ0OZK9A/edit?usp=sharing

  • cell_line

  • companalysis

  • contact

  • cv

  • db

  • expression

  • general

  • genetic

  • interaction

  • library

  • mage

  • map

  • natural_diversity

  • organism

  • phenotype

  • phylogeny

  • project

  • pub

  • sequence

  • stock

  • www

@spficklin

This comment has been minimized.

Copy link
Contributor

spficklin commented Jan 10, 2018

Let's consider adding this to all prop tables for consistency.

@scottcain

This comment has been minimized.

Copy link
Member

scottcain commented Jan 10, 2018

Adding this Chado-wide is a good idea, but is also a fairly wide-ranging suggested change (and would take a while to implement, if only because there are so many tables that would be effected). I'm not saying don't do this (in fact I'll reiterate that it's a really good idea), just that it will take a while :-)

@spficklin spficklin changed the title Add cvalue_id to biomaterialprop table Add cvalue_id to all prop tables Jan 10, 2018

@spficklin

This comment has been minimized.

Copy link
Contributor

spficklin commented Jan 10, 2018

Suggested code...

Add these lines to the prop table.

cvalue_id bigint,
FOREIGN KEY (cvalue_id) REFERENCES cvterm (cvterm_id) ON DELETE SET NULL,

Add this index:

CREATE INDEX {table}_idx{num} ON {table} (cvalue_id);

Add this comment for each prop table:

COMMENT ON COLUMN {table}.cvalue_id IS 'The value of the property if that value should be the name of a controlled vocabulary term.  It is preferred that a property either use the value or cvalue_id column but not both.  For example, if the property type is "color" then the cvalue_id could be a term named "green".';
@bradfordcondon

This comment has been minimized.

Copy link
Contributor

bradfordcondon commented Jan 10, 2018

We are going to skip cvtermprop tables (ie cell_line_cvtermprop). It sounds like this table accomplishes something similar to what we're doing with teh cvalue column but maybe not exactly the same.

We will still be adding cvalue to the prop tables for modules that have a cvtermprop table.

IE for cell_line we add cell_lineprop but ignore cell_line_cvtermprop

spficklin added a commit that referenced this issue Jan 10, 2018

This was referenced Jan 11, 2018

@laceysanderson

This comment has been minimized.

Copy link

laceysanderson commented Jan 13, 2018

Very excited about this change :-D

@bradfordcondon bradfordcondon referenced this issue Feb 5, 2018

Closed

CV targets #187

3 of 5 tasks complete
@bradfordcondon

This comment has been minimized.

Copy link
Contributor

bradfordcondon commented Jun 20, 2018

PR merged, closing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment