Add year to cxgn time ontology by ktmeaton · Pull Request #6100 · solgenomics/sgn

ktmeaton · 2026-05-11T16:24:00Z

Description

This pull requests adds time in years (year 1 to year 100) to the cxgn time ontology. In the config file, it can be enabled for compose traits and treatments by using the tiy config (time in years). I couldn't see any existing selenium tests for the compose interface, so I added some minimal tests under the tools category.

Resolves Adding year to the cxgn time ontology #6097

Checklist

ktmeaton · 2026-05-12T00:12:13Z

Failing test 44 of _BrAPIv2_phenotyping.t, related to brapi v2 traits. Possibly the new time components are throwing off a check of traits in the db.

ktmeaton · 2026-05-12T15:44:07Z

Yes, that's exactly what the problem was. I've now added 101 new 'traits' to the db, so the brapi expected count needs to be updated.

ktmeaton · 2026-05-12T20:31:11Z

CI passing, ready for review!

ryan-preble · 2026-05-14T20:23:05Z

+    # next available dbxref accession in time ontology is '0000481'
+    my $dbxref_accession = 481;


I'm a little wary of hardcoding the next highest accession number in an ontology - I will need to fish around to see if other databases have messed with their time ontologies in such a way that it would break this

That's a very good point, thank you. Initially I'd thought of using the autocreated: year 1 accession for the dbxref accession. But then I thought this might be inconsistent in a bad way, since all the other time ontology accessions have hard-coded numeric accessions. Another option might be to put it in its own ontology (ex. YEAR:000001). But that might also be consfusing if year is the only time unit to have its own ontology.

I'm curious to see what you find out in the other databases!

Hmm, maybe putting year in it's own time ontology was a bad suggestion. I imagine that will over complicate things downstream to have time in multiple ontologies (ex. plotting post-composed time traits #6096).

I think your general structure of placing a new root term in the TIME ontology and then assigning that root term to be a TIY composable ontology is the right approach. But I checked and it seems other breedbases have added additional time terms to their DB that throws off the accession numbers. I think the best approach would be to solve for the next highest accession number in the db patch and use that to form the root term, then start adding new accession numbers from that starting point. Changing sgn.conf to use 481 makes sense because sgn_local.conf can override that value for all the databases that have modified their TIME ontologies away from the default. We will just have to be careful to set the correct values in sgn_local.conf wherever relevant

Also, one more thing: I am pretty sure cvterm create_with() will ignore the definition parameter. When I wrote a dbpatch that created new ontologies, I had to manually update the definition after making it with create_with().

I've now changed the definition from inside create_with() to manual afterwards. Jumping over to Lukas' responses to discuss auto-detection of accession vs hard-coding a much larger root number.

lukasmueller · 2026-05-15T13:42:47Z

Year separate would be complicated. Hardcoding might be fine Get Outlook for iOS<https://aka.ms/o0ukef>

________________________________ From: Katherine Eaton ***@***.***> Sent: Thursday, May 14, 2026 3:34:09 PM To: solgenomics/sgn ***@***.***> Cc: Lukas A. Mueller ***@***.***>; Review requested ***@***.***> Subject: Re: [solgenomics/sgn] Add year to cxgn time ontology (PR #6100) @ktmeaton commented on this pull request.

________________________________ In db/00205/AddTimeOntologyYear.pm<#6100 (comment)>:

+ # next available dbxref accession in time ontology is '0000481'

+ my $dbxref_accession = 481; Hmm, maybe putting year in it's own time ontology was a bad suggestion. I imagine that will over complicate things downstream to have time in multiple ontologies (ex. plotting post-composed time traits #6096<#6096>). — Reply to this email directly, view it on GitHub<#6100 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AAAV7F4LFLWZTGEBOP2EYI342YUUDAVCNFSM6AAAAACYZMUATWVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHM2DEOJTGIZTMMBUHA>. Triage notifications on the go with GitHub Mobile for iOS<https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675> or Android<https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub>. You are receiving this because your review was requested.Message ID: ***@***.***>

lukasmueller · 2026-05-18T14:19:43Z

With hardcoding, I mean you predetermine what the IDs are, you don't let the script do it. That might lead to complications if the IDs are then not the same between systems.

lukasmueller · 2026-05-18T15:13:05Z

Start at 0005000 for the year terms or something like that so that they are likely available on all sytems

ktmeaton · 2026-05-19T15:45:57Z

Based on Lukas' comments, I've started the new time in years root to be at 0005000, with new entries going up to (and including 0005100) . @ryan-preble do you think this namespace is going to be compatible with most of the databases you've looked at so far?

ryan-preble · 2026-05-19T17:23:50Z

@@ -25,4 +25,4 @@
 # image_analysis_services { "plantcv_citrus_app": { "server_endpoint": "http://fake-image-analysis-service/", "image_type_name": "image_analysis_contours", "description": "Citrus Image Analysis (Mocked)", "service_traits": { "Fruit Diameter|INV:0000118": "INV" } } }

 composable_cvs trait,object,tod,toy,unit,method,experiment_treatment


Make sure tiy shows on this line too

Ah I missed that, thanks! Fixed in latest commit. I'm glad the CI tests correctly failed when I had that value set incorrectly.

ryan-preble · 2026-05-19T17:24:47Z

 # image_analysis_services { "plantcv_citrus_app": { "server_endpoint": "http://fake-image-analysis-service/", "image_type_name": "image_analysis_contours", "description": "Citrus Image Analysis (Mocked)", "service_traits": { "Fruit Diameter|INV:0000118": "INV" } } }

 composable_cvs trait,object,tod,toy,unit,method,experiment_treatment
 composable_cvs_allowed_combinations Agronomic|trait+toy,Metabolic|trait+object+tod+toy+unit+method


Question for @lukasmueller : do we want Time In Years to be available by default or suppressed until this line is overridden in sgn_local.conf?

ryan-preble · 2026-05-19T17:30:18Z

 composable_variables 1 #display only variable terms in the post composing tool
 composable_tod_root_cvterm "time of day|TIME:0000001"
 composable_toy_root_cvterm "time of year|TIME:0000005"
+composable_tiy_root_cvterm "time in years|TIME:0000481"


This needs to get bumped to 5000

Thanks, just updated now.

ktmeaton · 2026-05-19T19:35:17Z

Ah yes, CI is failing because the compose test is searching for specific time accession numbers to navigate the cvterm ontology tree in the UI. I will fix that test next.

Add year to cxgn time ontology

ac335dd

update expected trait count for brapi phenotyping test

2055101

ktmeaton mentioned this pull request May 12, 2026

Create a post composed traits plotting tool #6096

Open

titima15 requested review from lukasmueller and ryan-preble May 14, 2026 15:02

ryan-preble reviewed May 14, 2026

View reviewed changes

fix ontology definition and raise time root to 5000

4a2e663

ryan-preble self-requested a review May 19, 2026 17:01

ryan-preble reviewed May 19, 2026

View reviewed changes

ktmeaton added 2 commits May 19, 2026 11:27

update conf to have new time in years ontology root

44e9b49

add time in years to composable cvs

708f016

ryan-preble reviewed May 19, 2026

View reviewed changes

update compose trait to use new time in years accession

be88e1d

ryan-preble self-requested a review May 19, 2026 20:09

ryan-preble approved these changes May 19, 2026

View reviewed changes

		# next available dbxref accession in time ontology is '0000481'
		my $dbxref_accession = 481;

		@@ -25,4 +25,4 @@
		# image_analysis_services { "plantcv_citrus_app": { "server_endpoint": "http://fake-image-analysis-service/", "image_type_name": "image_analysis_contours", "description": "Citrus Image Analysis (Mocked)", "service_traits": { "Fruit Diameter\|INV:0000118": "INV" } } }

		composable_cvs trait,object,tod,toy,unit,method,experiment_treatment

Conversation

ktmeaton commented May 11, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

ktmeaton commented May 12, 2026

Uh oh!

ktmeaton commented May 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ktmeaton commented May 12, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryan-preble May 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lukasmueller commented May 15, 2026 via email

Uh oh!

lukasmueller commented May 18, 2026

Uh oh!

lukasmueller commented May 18, 2026

Uh oh!

ktmeaton commented May 19, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ryan-preble May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ktmeaton commented May 19, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ktmeaton commented May 11, 2026 •

edited

Loading

ktmeaton commented May 12, 2026 •

edited

Loading

ryan-preble May 15, 2026 •

edited

Loading

ryan-preble May 19, 2026 •

edited

Loading