Remove Compile warning in a case where we are subverting the type system by emckee2006 · Pull Request #91 · biojava/biojava

emckee2006 · 2014-02-10T06:53:28Z

ProteinSequence was using parentSequence as a DNA Sequence. However,
the field was parameterized to have the same CompoundSet for both the
sequence and its parent. Attempt to make this more explicit versus the
mostly silent compile warning which is hiding our breaking the type
system here.

Comments are welcome, as well as suggestions for better possible fixes to this issue..;

the field was parameterized to have the same CompoundSet for both the sequence and its parent. Attempt to make this more explicit versus the mostly silent compile warning which is hiding our breaking the type system here.

willishf · 2014-02-10T11:29:21Z

The intended purpose that was never properly implemented was to allow you
to go from the Protein Amino Acid Sequence to the mRNA sequence that codes
for it. If you were to load a ProteinSequence from Uniprot you would then
have the XML information that would point to the mRNA sequence at NCBI.
From that you should then be able to get the parent of the mRNA sequence
which(minus properly modeled intermediate sequences) would be the gene
sequence. From the gene sequence you should then be able to get the intron
exon sequences as children, as well as all known mRNA sequences. From the
known mRNA sequences you should then be able to get the known
ProteinSequences.

The pieces are in place for instantiating a ProteinSequence from a Uniprot
ID and NCBI sequences from an ID. Just hasn't been put together.

On Mon, Feb 10, 2014 at 1:53 AM, emckee2006 notifications@github.comwrote:

ProteinSequence was using parentSequence as a DNA Sequence. However,
the field was parameterized to have the same CompoundSet for both the
sequence and its parent. Attempt to make this more explicit versus the
mostly silent compile warning which is hiding our breaking the type
system here.

Comments are welcome, as well as suggestions for better possible fixes to

this issue..;

You can merge this Pull Request by running

git pull https://github.com/emckee2006/biojava master

Or view, comment on, or merge it at:

#91
Commit Summary

ProteinSequence was using parentSequence as a DNA Sequence. However,

File Changes

M
biojava3-core/src/main/java/org/biojava3/core/sequence/ProteinSequence.javahttps://github.com/Remove Compile warning in a case where we are subverting the type system #91/files#diff-0(5)

M
biojava3-core/src/main/java/org/biojava3/core/sequence/template/AbstractSequence.javahttps://github.com/Remove Compile warning in a case where we are subverting the type system #91/files#diff-1(12)

Patch Links:

https://github.com/biojava/biojava/pull/91.patch

https://github.com/biojava/biojava/pull/91.diff

Reply to this email directly or view it on GitHubhttps://github.com//pull/91
.

emckee2006 · 2014-02-10T16:05:20Z

That makes perfect sense, and is what I thought was supposed to happen
there eventually. However, if the type of the parent is tied to the type
of current sequence, then how can the protein sequence have a mRNA CDS as a
parent? (The generics should force the compound set of both sequences to
be the same)

On Mon, Feb 10, 2014 at 6:29 AM, willishf notifications@github.com wrote:

The intended purpose that was never properly implemented was to allow you
to go from the Protein Amino Acid Sequence to the mRNA sequence that codes
for it. If you were to load a ProteinSequence from Uniprot you would then
have the XML information that would point to the mRNA sequence at NCBI.
From that you should then be able to get the parent of the mRNA sequence
which(minus properly modeled intermediate sequences) would be the gene
sequence. From the gene sequence you should then be able to get the intron
exon sequences as children, as well as all known mRNA sequences. From the
known mRNA sequences you should then be able to get the known
ProteinSequences.

The pieces are in place for instantiating a ProteinSequence from a Uniprot
ID and NCBI sequences from an ID. Just hasn't been put together.

On Mon, Feb 10, 2014 at 1:53 AM, emckee2006 <notifications@github.com

wrote:

ProteinSequence was using parentSequence as a DNA Sequence. However,
the field was parameterized to have the same CompoundSet for both the
sequence and its parent. Attempt to make this more explicit versus the
mostly silent compile warning which is hiding our breaking the type
system here.

Comments are welcome, as well as suggestions for better possible fixes to

this issue..;

You can merge this Pull Request by running

git pull https://github.com/emckee2006/biojava master

Or view, comment on, or merge it at:

#91
Commit Summary

ProteinSequence was using parentSequence as a DNA Sequence. However,

File Changes

M

biojava3-core/src/main/java/org/biojava3/core/sequence/ProteinSequence.java<
https://github.com/biojava/biojava/pull/91/files#diff-0>(5)

M

biojava3-core/src/main/java/org/biojava3/core/sequence/template/AbstractSequence.java<
https://github.com/biojava/biojava/pull/91/files#diff-1>(12)

Patch Links:

https://github.com/biojava/biojava/pull/91.patch

https://github.com/biojava/biojava/pull/91.diff

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91>
.

Reply to this email directly or view it on GitHubhttps://github.com//pull/91#issuecomment-34621615
.

willishf · 2014-02-10T16:12:55Z

Didn't get far enough in the implementation to flush out the problems. The
challenge is with AbstractSequence as the common parent for all sequences
regardless of type. Nice to have a common parent for all generic/common
sequence related methods but may not be appropriate.

On Mon, Feb 10, 2014 at 11:05 AM, emckee2006 notifications@github.comwrote:

That makes perfect sense, and is what I thought was supposed to happen
there eventually. However, if the type of the parent is tied to the type
of current sequence, then how can the protein sequence have a mRNA CDS as a
parent? (The generics should force the compound set of both sequences to
be the same)

On Mon, Feb 10, 2014 at 6:29 AM, willishf notifications@github.com
wrote:

The intended purpose that was never properly implemented was to allow you
to go from the Protein Amino Acid Sequence to the mRNA sequence that
codes
for it. If you were to load a ProteinSequence from Uniprot you would then
have the XML information that would point to the mRNA sequence at NCBI.
From that you should then be able to get the parent of the mRNA sequence
which(minus properly modeled intermediate sequences) would be the gene
sequence. From the gene sequence you should then be able to get the
intron
exon sequences as children, as well as all known mRNA sequences. From the
known mRNA sequences you should then be able to get the known
ProteinSequences.

The pieces are in place for instantiating a ProteinSequence from a
Uniprot
ID and NCBI sequences from an ID. Just hasn't been put together.

On Mon, Feb 10, 2014 at 1:53 AM, emckee2006 <notifications@github.com

wrote:

ProteinSequence was using parentSequence as a DNA Sequence. However,
the field was parameterized to have the same CompoundSet for both the
sequence and its parent. Attempt to make this more explicit versus the
mostly silent compile warning which is hiding our breaking the type
system here.

Comments are welcome, as well as suggestions for better possible fixes
to

this issue..;

You can merge this Pull Request by running

git pull https://github.com/emckee2006/biojava master

Or view, comment on, or merge it at:

#91
Commit Summary

ProteinSequence was using parentSequence as a DNA Sequence. However,

File Changes

M

biojava3-core/src/main/java/org/biojava3/core/sequence/ProteinSequence.java<
https://github.com/biojava/biojava/pull/91/files#diff-0>(5)

M

biojava3-core/src/main/java/org/biojava3/core/sequence/template/AbstractSequence.java<
https://github.com/biojava/biojava/pull/91/files#diff-1>(12)

Patch Links:

https://github.com/biojava/biojava/pull/91.patch

https://github.com/biojava/biojava/pull/91.diff

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91>
.

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91#issuecomment-34621615>

.

Reply to this email directly or view it on GitHubhttps://github.com//pull/91#issuecomment-34648390
.

emckee2006 · 2014-02-10T16:16:01Z

If this fix takes care of that in a good way then it gets us one step
closer... What do you think of it?
On Feb 10, 2014 11:12 AM, "willishf" notifications@github.com wrote:

Didn't get far enough in the implementation to flush out the problems. The
challenge is with AbstractSequence as the common parent for all sequences
regardless of type. Nice to have a common parent for all generic/common
sequence related methods but may not be appropriate.

On Mon, Feb 10, 2014 at 11:05 AM, emckee2006 <notifications@github.com

wrote:

That makes perfect sense, and is what I thought was supposed to happen
there eventually. However, if the type of the parent is tied to the type
of current sequence, then how can the protein sequence have a mRNA CDS
as a
parent? (The generics should force the compound set of both sequences to
be the same)

On Mon, Feb 10, 2014 at 6:29 AM, willishf notifications@github.com
wrote:

The intended purpose that was never properly implemented was to allow
you
to go from the Protein Amino Acid Sequence to the mRNA sequence that
codes
for it. If you were to load a ProteinSequence from Uniprot you would
then
have the XML information that would point to the mRNA sequence at NCBI.
From that you should then be able to get the parent of the mRNA
sequence
which(minus properly modeled intermediate sequences) would be the gene
sequence. From the gene sequence you should then be able to get the
intron
exon sequences as children, as well as all known mRNA sequences. From
the
known mRNA sequences you should then be able to get the known
ProteinSequences.

The pieces are in place for instantiating a ProteinSequence from a
Uniprot
ID and NCBI sequences from an ID. Just hasn't been put together.

On Mon, Feb 10, 2014 at 1:53 AM, emckee2006 <notifications@github.com

wrote:

ProteinSequence was using parentSequence as a DNA Sequence. However,
the field was parameterized to have the same CompoundSet for both the
sequence and its parent. Attempt to make this more explicit versus
the
mostly silent compile warning which is hiding our breaking the type
system here.

Comments are welcome, as well as suggestions for better possible
fixes
to

this issue..;

You can merge this Pull Request by running

git pull https://github.com/emckee2006/biojava master

Or view, comment on, or merge it at:

#91
Commit Summary

ProteinSequence was using parentSequence as a DNA Sequence.
However,

File Changes

M

biojava3-core/src/main/java/org/biojava3/core/sequence/ProteinSequence.java<

https://github.com/biojava/biojava/pull/91/files#diff-0>(5)

M

biojava3-core/src/main/java/org/biojava3/core/sequence/template/AbstractSequence.java<

https://github.com/biojava/biojava/pull/91/files#diff-1>(12)

Patch Links:

https://github.com/biojava/biojava/pull/91.patch

https://github.com/biojava/biojava/pull/91.diff

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91>
.

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91#issuecomment-34621615>

.

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91#issuecomment-34648390>
.

Reply to this email directly or view it on GitHubhttps://github.com//pull/91#issuecomment-34649173
.

willishf · 2014-02-10T16:19:41Z

I think it will work. Won't know until I or someone has a reason to
actually try and do an implementation. We should plan another retreat
around a conference or block of group coding time to put in additional
functionality in core.

On Mon, Feb 10, 2014 at 11:16 AM, emckee2006 notifications@github.comwrote:

If this fix takes care of that in a good way then it gets us one step
closer... What do you think of it?

On Feb 10, 2014 11:12 AM, "willishf" notifications@github.com wrote:

Didn't get far enough in the implementation to flush out the problems.
The
challenge is with AbstractSequence as the common parent for all sequences
regardless of type. Nice to have a common parent for all generic/common
sequence related methods but may not be appropriate.

On Mon, Feb 10, 2014 at 11:05 AM, emckee2006 <notifications@github.com

wrote:

That makes perfect sense, and is what I thought was supposed to happen
there eventually. However, if the type of the parent is tied to the
type
of current sequence, then how can the protein sequence have a mRNA CDS
as a
parent? (The generics should force the compound set of both sequences
to
be the same)

On Mon, Feb 10, 2014 at 6:29 AM, willishf notifications@github.com
wrote:

The intended purpose that was never properly implemented was to allow
you
to go from the Protein Amino Acid Sequence to the mRNA sequence that
codes
for it. If you were to load a ProteinSequence from Uniprot you would
then
have the XML information that would point to the mRNA sequence at
NCBI.
From that you should then be able to get the parent of the mRNA
sequence
which(minus properly modeled intermediate sequences) would be the
gene
sequence. From the gene sequence you should then be able to get the
intron
exon sequences as children, as well as all known mRNA sequences. From
the
known mRNA sequences you should then be able to get the known
ProteinSequences.

The pieces are in place for instantiating a ProteinSequence from a
Uniprot
ID and NCBI sequences from an ID. Just hasn't been put together.

On Mon, Feb 10, 2014 at 1:53 AM, emckee2006 <
notifications@github.com

wrote:

ProteinSequence was using parentSequence as a DNA Sequence.
However,
the field was parameterized to have the same CompoundSet for both
the
sequence and its parent. Attempt to make this more explicit versus
the
mostly silent compile warning which is hiding our breaking the type
system here.

Comments are welcome, as well as suggestions for better possible
fixes
to

this issue..;

You can merge this Pull Request by running

git pull https://github.com/emckee2006/biojava master

Or view, comment on, or merge it at:

#91
Commit Summary

ProteinSequence was using parentSequence as a DNA Sequence.
However,

File Changes

M

biojava3-core/src/main/java/org/biojava3/core/sequence/ProteinSequence.java<

https://github.com/biojava/biojava/pull/91/files#diff-0>(5)

M

biojava3-core/src/main/java/org/biojava3/core/sequence/template/AbstractSequence.java<

https://github.com/biojava/biojava/pull/91/files#diff-1>(12)

Patch Links:

https://github.com/biojava/biojava/pull/91.patch

https://github.com/biojava/biojava/pull/91.diff

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91>
.

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91#issuecomment-34621615>

.

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91#issuecomment-34648390>
.

Reply to this email directly or view it on GitHub<
https://github.com/biojava/biojava/pull/91#issuecomment-34649173>

.

Reply to this email directly or view it on GitHubhttps://github.com//pull/91#issuecomment-34649526
.

andreasprlic · 2014-02-10T16:53:33Z

Can you also add a junit test that makes sure you can actually attach a DNA sequence as a parent?
Thanks,
Andreas

andreasprlic · 2014-03-12T21:52:05Z

Hi, where are we regarding this pull request? Any chance to get a unit test that checks for this? I see some compile errors with the genome module in Travis. it does not look like a networking error. We should try to fix this before merging this in.

willishf · 2014-03-12T22:59:40Z

Andreas

Wasn't sure if this email is directed at me or the person who made the
change? Heading to Scotland tomorrow for conference/vacation and just got
back in town after a week of travel. Won't have time to look at this in the
next 10 days.

Scooter

On Wed, Mar 12, 2014 at 5:52 PM, Andreas Prlic notifications@github.comwrote:

Hi, where are we regarding this pull request? Any chance to get a unit
test that checks for this? I see some compile errors with the genome module
in Travis. it does not look like a networking error. We should try to fix
this before merging this in.

Reply to this email directly or view it on GitHubhttps://github.com//pull/91#issuecomment-37473471
.

andreasprlic · 2014-03-12T23:03:32Z

mainly just checking in. I'd be also ok with merging this without a new junit test, as long as the old junit tests compile...

willishf · 2014-03-12T23:20:15Z

I haven't looked at it beyond what was in the email discussion. If the old
junit tests compile works for me. Java compiler will enforce the rules
based on this type of change.

On Wed, Mar 12, 2014 at 7:03 PM, Andreas Prlic notifications@github.comwrote:

mainly just checking in. I'd be also ok with merging this without a new
junit test, as long as the old junit tests compile...

Reply to this email directly or view it on GitHubhttps://github.com//pull/91#issuecomment-37480157
.

Catch back up with biojava/master

Remove Compile warning in a case where we are subverting the type system

…ul with CompoundSets

emckee2006 added 2 commits March 18, 2014 12:12

Merge pull request #16 from biojava/master

9824913

Catch back up with biojava/master

Merge pull request #17 from biojava/master

d193131

Catch back up with biojava/master

andreasprlic added a commit that referenced this pull request Mar 19, 2014

Merge pull request #91 from emckee2006/master

75ff1ab

Remove Compile warning in a case where we are subverting the type system

andreasprlic merged commit 75ff1ab into biojava:master Mar 19, 2014

andreasprlic added a commit to andreasprlic/biojava that referenced this pull request Mar 20, 2014

for the fix for biojava#91 to work I believe we need to be more caref…

5210929

…ul with CompoundSets

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Remove Compile warning in a case where we are subverting the type system#91

Remove Compile warning in a case where we are subverting the type system#91
andreasprlic merged 3 commits into
biojava:masterfrom
emckee2006:master

emckee2006 commented Feb 10, 2014

Uh oh!

willishf commented Feb 10, 2014

this issue..;

Uh oh!

emckee2006 commented Feb 10, 2014

this issue..;

Uh oh!

willishf commented Feb 10, 2014

this issue..;

Uh oh!

emckee2006 commented Feb 10, 2014

this issue..;

Uh oh!

willishf commented Feb 10, 2014

this issue..;

Uh oh!

andreasprlic commented Feb 10, 2014

Uh oh!

andreasprlic commented Mar 12, 2014

Uh oh!

willishf commented Mar 12, 2014

Uh oh!

andreasprlic commented Mar 12, 2014

Uh oh!

willishf commented Mar 12, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

emckee2006 commented Feb 10, 2014

Uh oh!

willishf commented Feb 10, 2014

this issue..;

Uh oh!

emckee2006 commented Feb 10, 2014

this issue..;

Uh oh!

willishf commented Feb 10, 2014

this issue..;

Uh oh!

emckee2006 commented Feb 10, 2014

this issue..;

Uh oh!

willishf commented Feb 10, 2014

this issue..;

Uh oh!

andreasprlic commented Feb 10, 2014

Uh oh!

andreasprlic commented Mar 12, 2014

Uh oh!

willishf commented Mar 12, 2014

Uh oh!

andreasprlic commented Mar 12, 2014

Uh oh!

willishf commented Mar 12, 2014

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants