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.

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

Feature/add alignment cache #1243

Merged
merged 1 commit into from
Sep 11, 2019
Merged

Conversation

rrahn
Copy link
Contributor

@rrahn rrahn commented Sep 3, 2019

Adds little helper class for the alignment cache instead of using a tuple which is hard to read since we can't use expressive namings for the variables.

NOTE: Only the last commit is relevant.

@rrahn rrahn requested a review from svnbgnk September 3, 2019 22:13
@rrahn rrahn force-pushed the feature/add_alignment_cache branch 2 times, most recently from 4a50eab to 5913290 Compare September 4, 2019 07:55
@codecov
Copy link

codecov bot commented Sep 4, 2019

Codecov Report

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

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1243   +/-   ##
=======================================
  Coverage   97.36%   97.36%           
=======================================
  Files         221      221           
  Lines        8912     8912           
=======================================
  Hits         8677     8677           
  Misses        235      235

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 2c42a07...a913963. Read the comment docs.

Copy link
Contributor

@svnbgnk svnbgnk left a comment

Choose a reason for hiding this comment

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

@rrahn rrahn force-pushed the feature/add_alignment_cache branch from 5913290 to 9b2c798 Compare September 9, 2019 14:14
@rrahn rrahn force-pushed the feature/add_alignment_cache branch from 9b2c798 to a913963 Compare September 10, 2019 15:56
@rrahn rrahn requested a review from smehringer September 10, 2019 16:26
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.

It is weird to merge something without a use case but I guess you will refactor this later on.

@rrahn
Copy link
Contributor Author

rrahn commented Sep 11, 2019

It is weird to merge something without a use case but I guess you will refactor this later on.

Well, no one want's to review a >2k LOC change either so at some level I have to make a compromise 🤷‍♂

@rrahn
Copy link
Contributor Author

rrahn commented Sep 11, 2019

The expressive names could be used in the unit test here: https://github.com/seqan/seqan3/blob/5913290cf1b38e9e488a42a571d5a6c48873641b/test/unit/alignment/pairwise/policy/affine_gap_banded_init_policy_test.cpp#L98-99.

A lot of these parts will be removed when I am done with the whole refactoring.

@rrahn rrahn merged commit e2c5fec into seqan:master Sep 11, 2019
@rrahn rrahn deleted the feature/add_alignment_cache branch October 11, 2019 17:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants