From: Diane Trout Date: Tue, 3 Apr 2007 23:39:44 +0000 (+0000) Subject: fix problems with motif changes not showing up in sequencebrowser X-Git-Url: http://woldlab.caltech.edu/gitweb/?p=mussa.git;a=commitdiff_plain;h=8b38b5bc63e5c62983d0814aa75d3f88b9116e49 fix problems with motif changes not showing up in sequencebrowser This patch adds a couple of new unit test cases to make sure that when the motif sequence list is changed it is actually reflected in the sequence browser. The problem was that when I originally coded it, a GlSequence held a shared_ptr so even though SequenceBrowser had new copies of GlSequence, the underlying Sequence was still shared. The change to GlSequence being a subclass of Sequence with a copy of the motif_list meant that changes to the motif_list in one copy weren't reflected in the copy held by the SequenceBrowser. I fixed it by changing sequence to hold a shared_ptr >. the downside is that that the motif_list is now always shared between copies of a Sequence, which is likely to cause problems when opening a second mussa window. However since I plan on tossing the current annotation and motif handling code at some point in the near future, this patch should be "good enough". --- diff --git a/alg/annotation_colors.hpp b/alg/annotation_colors.hpp index eaabc17..50c738b 100644 --- a/alg/annotation_colors.hpp +++ b/alg/annotation_colors.hpp @@ -4,6 +4,8 @@ #include #include +#include + #include "color.hpp" #include "sequence.hpp" @@ -24,6 +26,9 @@ struct DefaultColorMap color_map_type cm; }; +class AnnotationColors; +typedef boost::shared_ptr AnnotationColorsRef; + class AnnotationColors { public: diff --git a/alg/glseqbrowser.cpp b/alg/glseqbrowser.cpp index 03afaf5..d04f6f3 100644 --- a/alg/glseqbrowser.cpp +++ b/alg/glseqbrowser.cpp @@ -308,14 +308,14 @@ double GlSeqBrowser::zoom() const return zoom_level; } -void GlSeqBrowser::setColorMapper(boost::shared_ptr cm) +void GlSeqBrowser::setColorMapper(AnnotationColorsRef cm) { color_mapper = cm; } -const AnnotationColors& GlSeqBrowser::colorMapper() +const AnnotationColorsRef GlSeqBrowser::colorMapper() { - return *color_mapper; + return color_mapper; } void GlSeqBrowser::clear() @@ -362,7 +362,7 @@ void GlSeqBrowser::push_sequence(GlSequenceRef gs) path_segments.push_back(pair_segment_map()); } -const std::vector >& GlSeqBrowser::sequences() const +const std::vector& GlSeqBrowser::sequences() const { return track_container; } diff --git a/alg/glseqbrowser.hpp b/alg/glseqbrowser.hpp index 38ede4b..dcbf4a6 100644 --- a/alg/glseqbrowser.hpp +++ b/alg/glseqbrowser.hpp @@ -69,7 +69,7 @@ public: void centerOnPath(const std::vector&); void setColorMapper(boost::shared_ptr cm); - const AnnotationColors& colorMapper(); + const AnnotationColorsRef colorMapper(); //! clear our tracks and connections void clear(); @@ -208,13 +208,13 @@ private: //! the center of our current viewport (world coord) (used for scrollbar) float viewport_center; double zoom_level; - boost::shared_ptr color_mapper; + AnnotationColorsRef color_mapper; //! counter for each path added to us via connect int pathid; protected: //! container of all the GlSequences loaded into our scene - std::vector > track_container; + std::vector track_container; //! where to draw our box (world coordinates) rect selectedRegion; //! true if we have a selection diff --git a/alg/glsequence.cpp b/alg/glsequence.cpp index 17fb561..eb6cfe9 100644 --- a/alg/glsequence.cpp +++ b/alg/glsequence.cpp @@ -257,7 +257,7 @@ void GlSequence::draw_annotations(GLfloat left, GLfloat right) const // draw annotations GLfloat annotation_z = z() + 10.0; const std::list& annots = Sequence::annotations(); - const std::list& motifs = Sequence::motifs(); + const MotifList& motifs = Sequence::motifs(); for (std::list::const_iterator annot_itor = annots.begin(); annot_itor != annots.end(); ++annot_itor) @@ -267,7 +267,7 @@ void GlSequence::draw_annotations(GLfloat left, GLfloat right) const height(), annotation_z); } // if motifs? - for (std::list::const_iterator motifs_itor = motifs.begin(); + for (MotifList::const_iterator motifs_itor = motifs.begin(); motifs_itor != motifs.end(); ++motifs_itor) { @@ -275,7 +275,6 @@ void GlSequence::draw_annotations(GLfloat left, GLfloat right) const draw_box(left, right, x()+motifs_itor->begin, x()+motifs_itor->end, height(), annotation_z+1.0); } - } // this way of drawing characters, came from the red open gl book diff --git a/alg/glsequence.hpp b/alg/glsequence.hpp index cbb933c..4d93d15 100644 --- a/alg/glsequence.hpp +++ b/alg/glsequence.hpp @@ -54,6 +54,8 @@ public: const ColorRef color() const; //! return our draw color ColorRef color(); + //! return our annotation color mapper + AnnotationColorsRef colorMapper() { return color_mapper; } //! draw a track /*! left and right are the edges of the current viewport @@ -100,7 +102,7 @@ public: friend bool operator==(const GlSequence &left, const GlSequence &right); protected: - boost::shared_ptr color_mapper; + AnnotationColorsRef color_mapper; const GLfloat char_pix_per_world_unit; //! Return the pixel width of the opengl viewport. diff --git a/alg/mussa.cpp b/alg/mussa.cpp index 4b96d1b..f75290c 100644 --- a/alg/mussa.cpp +++ b/alg/mussa.cpp @@ -835,7 +835,7 @@ void Mussa::update_sequences_motifs() { // once we've loaded all the motifs from the file, // lets attach them to the sequences - for(vector >::iterator seq_i = the_seqs.begin(); + for(vector::iterator seq_i = the_seqs.begin(); seq_i != the_seqs.end(); ++seq_i) { diff --git a/alg/sequence.cpp b/alg/sequence.cpp index 608ddd3..ecbc633 100644 --- a/alg/sequence.cpp +++ b/alg/sequence.cpp @@ -90,7 +90,8 @@ Sequence::~Sequence() Sequence::Sequence(const char *seq, AlphabetRef alphabet_, SeqSpan::strand_type strand_) : header(""), - species("") + species(""), + motif_list(new MotifList) { set_filtered_sequence(seq, alphabet_, 0, npos, strand_); } @@ -99,7 +100,8 @@ Sequence::Sequence(const std::string& seq, AlphabetRef alphabet_, SeqSpan::strand_type strand_) : header(""), - species("") + species(""), + motif_list(new MotifList) { set_filtered_sequence(seq, alphabet_, 0, seq.size(), strand_); } @@ -134,7 +136,8 @@ Sequence::Sequence(const SequenceRef o) Sequence::Sequence(const SeqSpanRef& seq_ref) : seq(seq_ref), header(""), - species("") + species(""), + motif_list(new MotifList) { } @@ -637,7 +640,7 @@ Sequence::clear() header.clear(); species.clear(); annots.clear(); - motif_list.clear(); + motif_list.reset(new MotifList); } void @@ -755,18 +758,21 @@ void Sequence::add_motif(const Sequence& a_motif) motif_start_i != motif_starts.end(); ++motif_start_i) { - motif_list.push_back(motif(*motif_start_i, a_motif.get_sequence())); + motif_list->push_back(motif(*motif_start_i, a_motif.get_sequence())); } } void Sequence::clear_motifs() { - motif_list.clear(); + if (motif_list) + motif_list->clear(); + else + motif_list.reset(new MotifList); } -const std::list& Sequence::motifs() const +const Sequence::MotifList& Sequence::motifs() const { - return motif_list; + return *motif_list; } std::vector diff --git a/alg/sequence.hpp b/alg/sequence.hpp index fbd753f..9c7ddd4 100644 --- a/alg/sequence.hpp +++ b/alg/sequence.hpp @@ -105,6 +105,9 @@ public: typedef SeqString::const_reference const_reference; typedef SeqString::size_type size_type; static const size_type npos = SeqString::npos; + + typedef std::list MotifList; + typedef boost::shared_ptr MotifListRef; Sequence(AlphabetRef a = reduced_dna_alphabet); Sequence(const char* seq, @@ -225,7 +228,7 @@ public: //! add an annotation to our list of annotations void add_annotation(const annot& a); const std::list& annotations() const; - const std::list& motifs() const; + const MotifList& motifs() const; //! add a motif to our list of motifs void add_motif(const Sequence& a_motif); @@ -251,7 +254,7 @@ protected: //! store our oldstyle annotations std::list annots; //! a seperate list for motifs since we're currently not saving them - std::list motif_list; + MotifListRef motif_list; //! copy over all our annotation children void copy_children(Sequence &, size_type start, size_type count) const; diff --git a/alg/test/test_glseqbrowser.cpp b/alg/test/test_glseqbrowser.cpp index bbed062..acd450e 100644 --- a/alg/test/test_glseqbrowser.cpp +++ b/alg/test/test_glseqbrowser.cpp @@ -9,9 +9,10 @@ using namespace boost::assign; #include #include -#include "alg/annotation_colors.hpp" -#include "alg/glseqbrowser.hpp" -#include "alg/sequence.hpp" +#include "annotation_colors.hpp" +#include "glseqbrowser.hpp" +#include "mussa.hpp" +#include "sequence.hpp" using namespace std; @@ -157,7 +158,7 @@ BOOST_AUTO_TEST_CASE( setSelectedPaths ) Sequence seq0(s0); Sequence seq1(s1); Sequence seq2(s2); - + GlSeqBrowser gt; gt.push_sequence(seq0); gt.push_sequence(seq1); @@ -195,7 +196,6 @@ BOOST_AUTO_TEST_CASE( setSelectedTracks ) gt.push_sequence(seq1); gt.push_sequence(seq2); - BOOST_CHECK_EQUAL( gt.selectedTracks().size(), 0 ); gt.appendSelectedTrack(0, 0, seq0.size()); gt.appendSelectedTrack(2, 0, seq2.size()); @@ -218,5 +218,4 @@ BOOST_AUTO_TEST_CASE( setSelectedTracks ) BOOST_CHECK_EQUAL(seq_locs.front().getSequence(), seq0); BOOST_CHECK_EQUAL(seq_locs.front().getLeft(), 0); BOOST_CHECK_EQUAL(seq_locs.front().getRight(), 2); - -} +} \ No newline at end of file diff --git a/alg/test/test_sequence.cpp b/alg/test/test_sequence.cpp index 38852a5..921c5ef 100644 --- a/alg/test/test_sequence.cpp +++ b/alg/test/test_sequence.cpp @@ -33,6 +33,24 @@ BOOST_AUTO_TEST_CASE( sequence_copy_constructor ) BOOST_CHECK_EQUAL(s->get_species(), "foo"); BOOST_CHECK_EQUAL(c->get_species(), "bar"); } + +BOOST_AUTO_TEST_CASE( sequence_copy_constructor_copy_motifs ) +{ + SequenceRef s(new Sequence("AAAAGGGGAAAA")); + s->add_motif("AAGG"); + BOOST_CHECK_EQUAL(s->motifs().size(), 1); + + SequenceRef c(new Sequence(s->subseq())); + BOOST_CHECK_EQUAL(c->motifs().size(), 1); + + s->clear_motifs(); + BOOST_CHECK_EQUAL(s->motifs().size(), 0); + // FIXME: Technically c shouldn't lose its motifs. + // FIXME: getting that to work is hard. + // BOOST_CHECK_EQUAL(c->motifs().size(), 1); + BOOST_CHECK_EQUAL(c->motifs().size(), 0); +} + BOOST_AUTO_TEST_CASE( sequence_get_sequence ) { Sequence s; diff --git a/qui/motif_editor/test/TestMotifEditor.hpp b/qui/motif_editor/test/TestMotifEditor.hpp index 273473a..af01cb8 100644 --- a/qui/motif_editor/test/TestMotifEditor.hpp +++ b/qui/motif_editor/test/TestMotifEditor.hpp @@ -60,6 +60,57 @@ private slots: QVERIFY(sequences2[0]->motifs().size() == 1); QVERIFY(sequences2[1]->motifs().size() == 0); QVERIFY(cm->lookup("motif", "AAGG") == Color(0.0, 1.0, 0.0)); + QVERIFY(m->colorMapper()->lookup("motif", "AAGG") == Color(0.0, 1.0, 0.0)); + + QVERIFY(editor->get_analysis() == m); + QVERIFY(editor->get_model() == model); + QVERIFY(m->colorMapper() == cm); + } + + void testEditMotifApply() { + std::vector motifs; + motifs.push_back("AAGG"); + std::vector colors; + colors.push_back(Color(0.0, 0.0, 1.0)); + + MussaRef m(new Mussa); + m->append_sequence("AAAAGGGG"); + m->append_sequence("AAAACCCC"); + m->set_motifs(motifs, colors); + + // do the sequences have the right motifs? + std::vector sequences1(m->sequences()); + QVERIFY(sequences1[0]->get_sequence() == "AAAAGGGG"); + QVERIFY(sequences1[1]->get_sequence() == "AAAACCCC"); + QVERIFY(sequences1[0]->motifs().size() == 1); + QVERIFY(sequences1[1]->motifs().size() == 0); + + // get color + boost::shared_ptr cm(m->colorMapper()); + QVERIFY(cm->lookup("motif", "AAGG") == Color(0.0, 0.0, 1.0)); + + MotifEditor *editor(new MotifEditor(m)); + MotifModel *model(editor->get_model()); + // the AATT motif and the empty motif + QVERIFY(model->size() == 2); + QModelIndex row0_color(model->index(0, MotifModel::ColorCell)); + QModelIndex row0_sequence(model->index(0, MotifModel::SequenceCell)); + QVERIFY(model->data(row0_sequence) == "AAGG"); + + // change contents of cell + model->setData(row0_sequence, QVariant("AACC")); + // this is equivalent to pressing apply + editor->updateAnalysisMotifs(); + QVERIFY(model->data(row0_sequence) == "AACC"); + + // does the mussa analysis have the right data? + std::vector sequences2(m->sequences()); + QVERIFY(sequences2[0]->motifs().size() == 0); + QVERIFY(sequences2[1]->motifs().size() == 1); + QVERIFY(cm->lookup("motif", "AACC") == Color(0.0, 0.0, 1.0)); + QVERIFY(m->colorMapper()->lookup("motif", "AACC") == Color(0.0, 0.0, 1.0)); + QVERIFY(cm->lookup("motif", "AAGG") == Color(0.0, 0.0, 0.0)); + QVERIFY(m->colorMapper()->lookup("motif", "AAGG") == Color(0.0, 0.0, 0.0)); QVERIFY(editor->get_analysis() == m); QVERIFY(editor->get_model() == model); diff --git a/qui/seqbrowser/SequenceBrowserWidget.cpp b/qui/seqbrowser/SequenceBrowserWidget.cpp index c10136e..7ccf966 100644 --- a/qui/seqbrowser/SequenceBrowserWidget.cpp +++ b/qui/seqbrowser/SequenceBrowserWidget.cpp @@ -70,7 +70,6 @@ QSize SequenceBrowserWidget::sizeHint() const return QSize(width, center.height()); } - void SequenceBrowserWidget::clear() { converted_sequences.clear(); @@ -122,17 +121,17 @@ void SequenceBrowserWidget::copySelectedTracksAsSeqLocation( } void SequenceBrowserWidget::setSequences( - const std::vector >& sequences, - boost::shared_ptr cm) + const std::vector& new_sequences, + AnnotationColorsRef cm) { SequenceBrowser& browser = scrollable_browser->browser(); clear(); - for(Mussa::vector_sequence_type::const_iterator seq_i = sequences.begin(); - seq_i != sequences.end(); + for(Mussa::vector_sequence_type::const_iterator seq_i = new_sequences.begin(); + seq_i != new_sequences.end(); ++seq_i) { // Blech *(*seq_i) is dereferencing the shared_ptr stored in the iterator. - boost::shared_ptr gs(new GlSequence(*(*seq_i), cm)); + boost::shared_ptr gs(new GlSequence(*seq_i, cm)); converted_sequences.push_back(gs); browser.push_sequence(gs); } @@ -142,7 +141,7 @@ void SequenceBrowserWidget::setSequences( // connect the text change signals to each other SequenceBrowserSidebar::collection left = left_sidebar->descriptions; SequenceBrowserSidebar::collection right = right_sidebar->descriptions; - for(size_t i = 0; i != sequences.size() and i != right.size(); ++i) + for(size_t i = 0; i != new_sequences.size() and i != right.size(); ++i) { connect(left[i], SIGNAL(nameChanged(const QString &)), right[i], SLOT(setName(const QString &))); diff --git a/qui/seqbrowser/SequenceBrowserWidget.hpp b/qui/seqbrowser/SequenceBrowserWidget.hpp index 2b47d41..9b4e9c7 100644 --- a/qui/seqbrowser/SequenceBrowserWidget.hpp +++ b/qui/seqbrowser/SequenceBrowserWidget.hpp @@ -22,6 +22,9 @@ class SequenceBrowserWidget : public QScrollArea public: SequenceBrowserWidget(boost::shared_ptr, QWidget *parent=0); + //! return our wrapped browser, so we can test it. + SequenceBrowser& browser() { return scrollable_browser->browser(); } + //! return the popup menu for the glcanvas (reference stored internally) QMenu *getPopupMenu(); //! return our fasta copy action (reference stored internally) @@ -93,6 +96,6 @@ private: //! sequences created by a setSequences(vector) call /*! I need to save them so i can free them to avoid a memory leak */ - std::vector > converted_sequences; + std::vector converted_sequences; }; #endif diff --git a/qui/seqbrowser/test/CMakeLists.txt b/qui/seqbrowser/test/CMakeLists.txt index 73aeb2a..5b61c8f 100644 --- a/qui/seqbrowser/test/CMakeLists.txt +++ b/qui/seqbrowser/test/CMakeLists.txt @@ -2,4 +2,5 @@ INCLUDE( TestMacros ) MAKE_QUI_UNITTEST(TestSequenceBrowser) +MAKE_QUI_UNITTEST(TestSequenceBrowserWidget) MAKE_QUI_UNITTEST(TestSequenceDescription) \ No newline at end of file diff --git a/qui/seqbrowser/test/TestSequenceBrowserWidget.cpp b/qui/seqbrowser/test/TestSequenceBrowserWidget.cpp new file mode 100644 index 0000000..a08933d --- /dev/null +++ b/qui/seqbrowser/test/TestSequenceBrowserWidget.cpp @@ -0,0 +1,3 @@ +#include "TestSequenceBrowserWidget.hpp" + +QTEST_MAIN(TestSequenceBrowserWidget) diff --git a/qui/seqbrowser/test/TestSequenceBrowserWidget.hpp b/qui/seqbrowser/test/TestSequenceBrowserWidget.hpp new file mode 100644 index 0000000..fc82648 --- /dev/null +++ b/qui/seqbrowser/test/TestSequenceBrowserWidget.hpp @@ -0,0 +1,116 @@ +#ifndef TESTSEQUENCEBROWSERWIDGET_HPP_ +#define TESTSEQUENCEBROWSERWIDGET_HPP_ +#include +#include +#include +#include + +#include + +#include + +#include "alg/sequence.hpp" +#include "qui/seqbrowser/SequenceBrowserWidget.hpp" + +class TestSequenceBrowserWidget : public QObject +{ + Q_OBJECT + +private slots: + + void testSimpleConstructor() { + boost::shared_ptr d(new QDir(".")); + SequenceBrowserWidget *browser = new SequenceBrowserWidget(d); + } + + // do we have the right colors? + void testChangeMotifColor() { + boost::shared_ptr d(new QDir(".")); + + std::vector motifs; + motifs.push_back("AAGG"); + std::vector colors1; + colors1.push_back(Color(0.0, 0.0, 1.0)); + + MussaRef m(new Mussa); + m->append_sequence("AAAAGGGGTTTT"); + m->set_motifs(motifs, colors1); + QVERIFY(m->sequences().size() == 1); + QVERIFY(m->sequences()[0]->motifs().size() == 1); + QVERIFY(m->sequences()[0]->motifs().front().sequence == "AAGG"); + QVERIFY(m->colorMapper()->lookup("motif", "AAGG") == colors1[0]); + + SequenceBrowserWidget *browser = new SequenceBrowserWidget(d); + browser->setSequences(m->sequences(), m->colorMapper()); + std::vector glseqs1(browser->sequences()); + QVERIFY(glseqs1.size() == 1); + QVERIFY(glseqs1[0]->colorMapper() == m->colorMapper()); + QVERIFY(glseqs1[0]->colorMapper()->lookup("motif", "AAGG") == colors1[0]); + + std::vector colors2; + colors2.push_back(Color(1.0, 0.0, 0.0)); + m->set_motifs(motifs, colors2); + QVERIFY(m->sequences().size() == 1); + QVERIFY(m->sequences()[0]->motifs().size() == 1); + QVERIFY(m->sequences()[0]->motifs().front().sequence == "AAGG"); + QVERIFY(m->colorMapper()->lookup("motif", "AAGG") == colors2[0]); + + std::vector glseqs2(browser->sequences()); + QVERIFY(glseqs2.size() == 1); + QVERIFY(glseqs2[0]->colorMapper() == m->colorMapper()); + QVERIFY(glseqs2[0]->colorMapper()->lookup("motif", "AAGG") == colors2[0]); + } + + // do we have the right colors? + void testChangeMotifSequencer() { + boost::shared_ptr d(new QDir(".")); + + std::string aagg("AAGG"); + std::vector motifs1; + motifs1.push_back(aagg); + std::vector colors1; + colors1.push_back(Color(0.0, 0.0, 1.0)); + + MussaRef m(new Mussa); + m->append_sequence("AAAAGGGG"); + m->append_sequence("AAAACCCC"); + m->set_motifs(motifs1, colors1); + QVERIFY(m->sequences().size() == 2); + QVERIFY(m->sequences()[0]->motifs().size() == 1); + QVERIFY(m->sequences()[0]->motifs().front().sequence == aagg); + QVERIFY(m->sequences()[1]->motifs().size() == 0); + QVERIFY(m->colorMapper()->lookup("motif", aagg) == colors1[0]); + + SequenceBrowserWidget *browser = new SequenceBrowserWidget(d); + browser->setSequences(m->sequences(), m->colorMapper()); + // does the sequence browser have the right motif pattern? + std::vector glseqs1(browser->sequences()); + QVERIFY(glseqs1.size() == 2); + QVERIFY(glseqs1[0]->motifs().size() == 1); + QVERIFY(glseqs1[0]->motifs().front().sequence == aagg); + QVERIFY(glseqs1[1]->motifs().size() == 0); + QVERIFY(glseqs1[0]->colorMapper() == m->colorMapper()); + QVERIFY(glseqs1[0]->colorMapper()->lookup("motif", aagg) == colors1[0]); + + std::vector motifs2; + std::string aacc("AACC"); + motifs2.push_back(aacc); + m->set_motifs(motifs2, colors1); + // did the copy of our sequences in the mussa analysis object get updated? + QVERIFY(m->sequences().size() == 2); + QVERIFY(m->sequences()[0]->motifs().size() == 0); + QVERIFY(m->sequences()[1]->motifs().size() == 1); + QVERIFY(m->sequences()[1]->motifs().front().sequence == aacc); + QVERIFY(m->colorMapper()->lookup("motif", aacc) == colors1[0]); + + // did the copy of our sequences in the browser get updated? + std::vector glseqs2(browser->sequences()); + QVERIFY(glseqs2.size() == 2); + QVERIFY(glseqs2[0]->motifs().size() == 0); + QVERIFY(glseqs2[1]->motifs().size() == 1); + QVERIFY(glseqs2[1]->motifs().front().sequence == aacc); + QVERIFY(glseqs2[0]->colorMapper() == m->colorMapper()); + QVERIFY(glseqs2[0]->colorMapper()->lookup("motif", aacc) == colors1[0]); + } +}; +#endif /*TESTSEQUENCEBROWSERWIDGET_HPP_*/