poly icon indicating copy to clipboard operation
poly copied to clipboard

`Gff.AddFeature()` code is misleading and mutates `Feature` state

Open carreter opened this issue 2 years ago • 5 comments

Gff.AddFeature() seems to intend to create a copy of the Feature it is provided. Dereferencing the pointer is not sufficient to do this, as Feature structs contain mutable fields (e.g. Feature.Attribtues is a map[string]string, and Feature.Location is a struct that has a slice field in it).

https://github.com/TimothyStiles/poly/blob/ad662c741528bbb5bcaa7a3107991a86426c1e55/io/gff/gff.go#L78-L84

IMO, the Feature.ParentSequence field should be dropped entirely. Currently, it is only used by getFeatureSequence, which could easily be modified to be a method of the Gff struct. This makes more sense as Features and Locations only exist in the context of a Gff anyway.

This also means that the Gff.AddFeature() method can be entirely dropped in favor of directly appending it to Gff.Features.

It's a small change, so I'm more than happy to do this once given the go-ahead!

Blocked by #339 .

carreter avatar Sep 11 '23 23:09 carreter

I think I talked to @TimothyStiles about this one, and basically - yea its both a problem here AND in genbank. It'd probably be easiest to fix from #339 branch, so the merge will be easier. 100% we should make this better!

Koeng101 avatar Sep 14 '23 05:09 Koeng101

I'll wait until you merge #339 then do this.

carreter avatar Sep 15 '23 20:09 carreter

Status: Ready to merge :heavy_check_mark:

Issues blocking this PR:

  • #339 :heavy_check_mark:

This comment was automatically written by the Blocking Issues bot, and this PR will be monitored for further progress.

github-actions[bot] avatar Sep 23 '23 03:09 github-actions[bot]

This issue has had no activity in the past 2 months. Marking as stale.

github-actions[bot] avatar Nov 22 '23 18:11 github-actions[bot]

This issue has had no activity in the past 2 months. Marking as stale.

github-actions[bot] avatar Feb 06 '24 18:02 github-actions[bot]