Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[MISC] remove using namespace seqan3/test/unit/alphabet/a*-c* #1657

Merged
merged 3 commits into from
Apr 6, 2020
Merged

[MISC] remove using namespace seqan3/test/unit/alphabet/a*-c* #1657

merged 3 commits into from
Apr 6, 2020

Conversation

Irallia
Copy link
Contributor

@Irallia Irallia commented Mar 19, 2020

Resolves #1650.

@Irallia Irallia requested review from a team and eseiler and removed request for a team March 19, 2020 15:57
@smehringer smehringer requested review from a team and simonsasse and removed request for eseiler and a team March 19, 2020 16:16
@codecov
Copy link

codecov bot commented Mar 19, 2020 •
edited
Loading

Codecov Report

Merging #1657 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1657   +/-   ##
=======================================
  Coverage   97.67%   97.67%           
=======================================
  Files         237      237           
  Lines        9045     9045           
=======================================
  Hits         8835     8835           
  Misses        210      210

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fac9ad9...0ffaa02. Read the comment docs.

@simonsasse simonsasse requested review from a team and removed request for simonsasse March 30, 2020 11:47
@Irallia Irallia requested review from sarahet, a team and joergi-w and removed request for a team and sarahet March 30, 2020 13:43
marehr
marehr reviewed Mar 30, 2020
View reviewed changes
Copy link
Member

@marehr marehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

I added some suggestions which remove a lot of unnecessary indentations and whitespaces.

test/unit/alphabet/adaptation/char_test.cpp Outdated Show resolved Hide resolved
test/unit/alphabet/adaptation/char_test.cpp Outdated Show resolved Hide resolved
test/unit/alphabet/adaptation/char_test.cpp Outdated Show resolved Hide resolved
test/unit/alphabet/adaptation/char_test.cpp Outdated Show resolved Hide resolved
test/unit/alphabet/adaptation/char_test.cpp Outdated Show resolved Hide resolved
test/unit/alphabet/composite/composite_integration_test.cpp Outdated
Comment on lines 335 to 337
EXPECT_LT(t41, seqan3::gap{}); // *
EXPECT_LT(t41, (qualified_dna_phred42{'G'_dna4, seqan3::phred42{2}})); // *
EXPECT_LT(t41, (gapped_qualified_dna_phred42{qualified_dna_phred42{'G'_dna4, seqan3::phred42{2}}}));
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EXPECT_LT(t41, seqan3::gap{}); // *
EXPECT_LT(t41, (qualified_dna_phred42{'G'_dna4, seqan3::phred42{2}})); // *
EXPECT_LT(t41, (gapped_qualified_dna_phred42{qualified_dna_phred42{'G'_dna4, seqan3::phred42{2}}}));
EXPECT_LT(t41, seqan3::gap{}); // *
EXPECT_LT(t41, (qualified_dna_phred42{'G'_dna4, seqan3::phred42{2}})); // *
EXPECT_LT(t41, (gapped_qualified_dna_phred42{qualified_dna_phred42{'G'_dna4, seqan3::phred42{2}}}));

test/unit/alphabet/composite/composite_integration_test.cpp Outdated
Comment on lines 339 to 348
EXPECT_EQ('C'_dna4, t41);
EXPECT_EQ('C'_rna4, t41);
EXPECT_EQ(seqan3::phred42{3}, t41);
EXPECT_EQ((qualified_dna_phred42{'C'_dna4, seqan3::phred42{3}}), t41);
EXPECT_NE(seqan3::gap{}, t41);
// EXPECT_LT('A'_dna4, t41); // not supposed to work
// EXPECT_LT('A'_rna4, t41); // not supposed to work
// EXPECT_LT(seqan3::phred42{2}, t41); // not supposed to work
EXPECT_LT((qualified_dna_phred42{'A'_dna4, seqan3::phred42{2}}), t41); // *
EXPECT_GT(seqan3::gap{}, t41); // *
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EXPECT_EQ('C'_dna4, t41);
EXPECT_EQ('C'_rna4, t41);
EXPECT_EQ(seqan3::phred42{3}, t41);
EXPECT_EQ((qualified_dna_phred42{'C'_dna4, seqan3::phred42{3}}), t41);
EXPECT_NE(seqan3::gap{}, t41);
// EXPECT_LT('A'_dna4, t41); // not supposed to work
// EXPECT_LT('A'_rna4, t41); // not supposed to work
// EXPECT_LT(seqan3::phred42{2}, t41); // not supposed to work
EXPECT_LT((qualified_dna_phred42{'A'_dna4, seqan3::phred42{2}}), t41); // *
EXPECT_GT(seqan3::gap{}, t41); // *
EXPECT_EQ('C'_dna4, t41);
EXPECT_EQ('C'_rna4, t41);
EXPECT_EQ(seqan3::phred42{3}, t41);
EXPECT_EQ((qualified_dna_phred42{'C'_dna4, seqan3::phred42{3}}), t41);
EXPECT_NE(seqan3::gap{}, t41);
// EXPECT_LT('A'_dna4, t41); // not supposed to work
// EXPECT_LT('A'_rna4, t41); // not supposed to work
// EXPECT_LT(seqan3::phred42{2}, t41); // not supposed to work
EXPECT_LT((qualified_dna_phred42{'A'_dna4, seqan3::phred42{2}}), t41); // *
EXPECT_GT(seqan3::gap{}, t41); // *

test/unit/alphabet/composite/composite_integration_test.cpp Outdated
Comment on lines 353 to 375
EXPECT_NE(t51, seqan3::gap{});
EXPECT_EQ(t51, seqan3::gapped<seqan3::dna4>('C'_dna4));
EXPECT_EQ(t51, seqan3::phred42{0}); // "outer" phred element
EXPECT_EQ(t51, (qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{3}}));
// EXPECT_LT(t51, 'G'_dna4); // not supposed to work
// EXPECT_LT(t51, 'G'_rna4); // not supposed to work
// EXPECT_LT(t51, seqan3::gap{}); // not supposed to work
// EXPECT_LT(t51, seqan3::gapped<seqan3::dna4>('G'_dna4)); // not supposed to work
EXPECT_LT(t51, seqan3::phred42{1});
EXPECT_LT(t51, (qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{4}}));

EXPECT_EQ('C'_dna4, t51);
EXPECT_EQ('C'_rna4, t51);
EXPECT_NE(seqan3::gap{}, t51);
EXPECT_EQ(seqan3::gapped<seqan3::dna4>('C'_dna4), t51);
EXPECT_EQ(seqan3::phred42{0}, t51);
EXPECT_EQ((qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{3}}), t51);
// EXPECT_LT('A'_dna4, t51); // not supposed to work
// EXPECT_LT('A'_rna4, t51); // not supposed to work
// EXPECT_GT(seqan3::gap{}, t51); // not supposed to work
// EXPECT_LT(seqan3::gapped<seqan3::dna4>('A'_dna4), t51); // not supposed to work
EXPECT_GT(seqan3::phred42{1}, t51);
EXPECT_GT((qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{4}}), t51);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EXPECT_NE(t51, seqan3::gap{});
EXPECT_EQ(t51, seqan3::gapped<seqan3::dna4>('C'_dna4));
EXPECT_EQ(t51, seqan3::phred42{0}); // "outer" phred element
EXPECT_EQ(t51, (qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{3}}));
// EXPECT_LT(t51, 'G'_dna4); // not supposed to work
// EXPECT_LT(t51, 'G'_rna4); // not supposed to work
// EXPECT_LT(t51, seqan3::gap{}); // not supposed to work
// EXPECT_LT(t51, seqan3::gapped<seqan3::dna4>('G'_dna4)); // not supposed to work
EXPECT_LT(t51, seqan3::phred42{1});
EXPECT_LT(t51, (qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{4}}));
EXPECT_EQ('C'_dna4, t51);
EXPECT_EQ('C'_rna4, t51);
EXPECT_NE(seqan3::gap{}, t51);
EXPECT_EQ(seqan3::gapped<seqan3::dna4>('C'_dna4), t51);
EXPECT_EQ(seqan3::phred42{0}, t51);
EXPECT_EQ((qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{3}}), t51);
// EXPECT_LT('A'_dna4, t51); // not supposed to work
// EXPECT_LT('A'_rna4, t51); // not supposed to work
// EXPECT_GT(seqan3::gap{}, t51); // not supposed to work
// EXPECT_LT(seqan3::gapped<seqan3::dna4>('A'_dna4), t51); // not supposed to work
EXPECT_GT(seqan3::phred42{1}, t51);
EXPECT_GT((qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{4}}), t51);
EXPECT_NE(t51, seqan3::gap{});
EXPECT_EQ(t51, seqan3::gapped<seqan3::dna4>('C'_dna4));
EXPECT_EQ(t51, seqan3::phred42{0}); // "outer" phred element
EXPECT_EQ(t51, (qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{3}}));
// EXPECT_LT(t51, 'G'_dna4); // not supposed to work
// EXPECT_LT(t51, 'G'_rna4); // not supposed to work
// EXPECT_LT(t51, seqan3::gap{}); // not supposed to work
// EXPECT_LT(t51, seqan3::gapped<seqan3::dna4>('G'_dna4)); // not supposed to work
EXPECT_LT(t51, seqan3::phred42{1});
EXPECT_LT(t51, (qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{4}}));
EXPECT_EQ('C'_dna4, t51);
EXPECT_EQ('C'_rna4, t51);
EXPECT_NE(seqan3::gap{}, t51);
EXPECT_EQ(seqan3::gapped<seqan3::dna4>('C'_dna4), t51);
EXPECT_EQ(seqan3::phred42{0}, t51);
EXPECT_EQ((qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{3}}), t51);
// EXPECT_LT('A'_dna4, t51); // not supposed to work
// EXPECT_LT('A'_rna4, t51); // not supposed to work
// EXPECT_GT(seqan3::gap{}, t51); // not supposed to work
// EXPECT_LT(seqan3::gapped<seqan3::dna4>('A'_dna4), t51); // not supposed to work
EXPECT_GT(seqan3::phred42{1}, t51);
EXPECT_GT((qualified_gapped_dna_phred42{'C'_dna4, seqan3::phred42{4}}), t51);

test/unit/alphabet/composite/composite_integration_test.cpp Outdated
Comment on lines 380 to 385
EXPECT_NE(t61, seqan3::gap{});
EXPECT_NE(t61, seqan3::phred42{0});
EXPECT_LT(t61, 'G'_rna4); // *
EXPECT_LT(t61, 'G'_dna4); // *
EXPECT_LT(t61, seqan3::gap{}); // *
EXPECT_LT(t61, seqan3::phred42{1}); // *
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EXPECT_NE(t61, seqan3::gap{});
EXPECT_NE(t61, seqan3::phred42{0});
EXPECT_LT(t61, 'G'_rna4); // *
EXPECT_LT(t61, 'G'_dna4); // *
EXPECT_LT(t61, seqan3::gap{}); // *
EXPECT_LT(t61, seqan3::phred42{1}); // *
EXPECT_NE(t61, seqan3::gap{});
EXPECT_NE(t61, seqan3::phred42{0});
EXPECT_LT(t61, 'G'_rna4); // *
EXPECT_LT(t61, 'G'_dna4); // *
EXPECT_LT(t61, seqan3::gap{}); // *
EXPECT_LT(t61, seqan3::phred42{1}); // *

test/unit/alphabet/composite/composite_integration_test.cpp Outdated
Comment on lines 387 to 394
EXPECT_EQ('C'_rna4, t61);
EXPECT_EQ('C'_dna4, t61);
EXPECT_NE(seqan3::gap{}, t61);
EXPECT_NE(seqan3::phred42{0}, t61);
EXPECT_LT('A'_rna4, t61); // *
EXPECT_LT('A'_dna4, t61); // *
EXPECT_GT(seqan3::gap{}, t61); // *
EXPECT_GT(seqan3::phred42{0}, t61); // *
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
EXPECT_EQ('C'_rna4, t61);
EXPECT_EQ('C'_dna4, t61);
EXPECT_NE(seqan3::gap{}, t61);
EXPECT_NE(seqan3::phred42{0}, t61);
EXPECT_LT('A'_rna4, t61); // *
EXPECT_LT('A'_dna4, t61); // *
EXPECT_GT(seqan3::gap{}, t61); // *
EXPECT_GT(seqan3::phred42{0}, t61); // *
EXPECT_EQ('C'_rna4, t61);
EXPECT_EQ('C'_dna4, t61);
EXPECT_NE(seqan3::gap{}, t61);
EXPECT_NE(seqan3::phred42{0}, t61);
EXPECT_LT('A'_rna4, t61); // *
EXPECT_LT('A'_dna4, t61); // *
EXPECT_GT(seqan3::gap{}, t61); // *
EXPECT_GT(seqan3::phred42{0}, t61); // *

@marehr marehr removed the request for review from joergi-w March 30, 2020 20:37
@Irallia Irallia force-pushed the misc/remove_using_namespace_seqan3/test_unit_alphabet_a-c branch from 6577155 to 0ffaa02 Compare March 31, 2020 09:13
@Irallia Irallia requested a review from marehr March 31, 2020 09:13
marehr
marehr approved these changes Apr 1, 2020
View reviewed changes
Copy link
Member

@marehr marehr left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you :)

@Irallia Irallia requested review from a team and removed request for a team April 2, 2020 10:41
@Irallia Irallia requested a review from smehringer April 2, 2020 10:42
smehringer
smehringer approved these changes Apr 6, 2020
View reviewed changes
Copy link
Member

@smehringer smehringer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@smehringer smehringer merged commit 97d7646 into seqan:master Apr 6, 2020
wvdtoorn pushed a commit to wvdtoorn/seqan3 that referenced this pull request Apr 7, 2020
@Irallia @wvdtoorn
[MISC] remove using namespace seqan3/test/unit/alphabet/a*-c* (seqan#…
dc98815 
…1657)

* [MISC] Remove 'using namespace seqan3' from test/unit/alphabet/a*.

* [MISC] Remove 'using namespace seqan3' from test/unit/alphabet/c*.

* [MISC] style guide fixes
wvdtoorn pushed a commit to wvdtoorn/seqan3 that referenced this pull request Apr 15, 2020
@Irallia @wvdtoorn
[MISC] remove using namespace seqan3/test/unit/alphabet/a*-c* (seqan#…
e83d954 
…1657)

* [MISC] Remove 'using namespace seqan3' from test/unit/alphabet/a*.

* [MISC] Remove 'using namespace seqan3' from test/unit/alphabet/c*.

* [MISC] style guide fixes
@Irallia Irallia mentioned this pull request Apr 24, 2020
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@smehringer smehringer smehringer approved these changes

@marehr marehr marehr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[MISC] Remove 'using namespace seqan3' from test/unit/alphabet/a* - c*
3 participants
@Irallia @marehr @smehringer