search

cgd / drake-genetics

An educational web application for exploring genetics using the fictional drake genome.
GNU General Public License v3.0
1 stars 0 forks source link

Meiosis engine #1

Closed keithshep closed 13 years ago

keithshep commented 14 years ago

Glen, sorry it seems that there is no way to not have you notified...

Dave, I'd like to give this a try for code reviews. If you have time, please add any comments you have to the pull request (no rush... within the next few days is fine). If you don't have time don't worry you can just apply the changes but at least it gives us both a chance to see how this works.

-Keith

keithshep commented 14 years ago

actually, don't worry about applying the changes. I assumed there would be a button for that on the page but it needs to be done by command line so I can do that after you've reviewed or said that you don't have time to review. Thanks

keithshep commented 13 years ago

I have some random comment to make about this line: http://github.com/cgd/drake-genetics/pull/1#L9R176

keithshep commented 13 years ago

OK second try. I had to click on the "view file" link to get to the file first: http://github.com/cgd/drake-genetics/blob/8adee6c7666b6d0ad265900117eaa86a59a682d9/modules/drake-genetics-client/src/java/org/jax/drakegenetics/shareddata/client/Chromosome.java#L156

keithshep commented 13 years ago

the Chromosome class is for representing an particular individual's chromosome. The ChromosomeDescription class is for representing the chromosome's structure for a species. I think the commit view may not be showing the best view of the merge request unfortunately since it's a fragmented view. For instance you weren't able to see the comment that I added to the top of Chromosome.java in that particular commit... but the comment will appear in under "Files Changed" which aggregates all of the commits for the merge request together... the one let down there is you cant directly comment on the lines of code in that view :(

obwalton commented 13 years ago

This is a test to see if I can be added as an active participant of this discussion.

obwalton commented 13 years ago

I don't fully understand the purpose of this method: http://github.com/cgd/drake-genetics/blob/8adee6c7666b6d0ad265900117eaa86a59a682d9/modules/drake-genetics-client/src/java/org/jax/drakegenetics/shareddata/client/Chromosome.java#L135

Am I right to assume this is method that will take all of a the chromosome in a list (possibly all of an individual's chromosomes), then put them into a hash by chromosome name, where they will be added to lists of many individual's chromosomes of the same name? Why would a user need this? Should this really be part of the Chromsome class? Or should it be part of a class that represents Chromosome for the species (ChromosomeDescription)? Or maybe some sort of utility class with methods for dealing with chromosomes of multiple individuals? The same question could probably be applied to addChromosomeMatches() as well.

keithshep commented 13 years ago