[MISC] Alignment: aminoacid scoring scheme - lower case member constants #2599

Irallia · 2021-05-04T13:12:00Z

vercel · 2021-05-04T13:12:04Z

This pull request is being automatically deployed with Vercel (learn more).
To see the status of your deployment, click below or on the icon next to each commit.

🔍 Inspect: https://vercel.com/seqan/seqan3/D173vmSyixxGMnKQ9EQAHzfG1fxy
✅ Preview: https://seqan3-git-fork-irallia-misc-alignmentaminoacids-6b3a04.vercel.app

codecov · 2021-05-04T14:28:10Z

Codecov Report

Merging #2599 (a4eb444) into master (cca92c4) will not change coverage.
The diff coverage is 100.00%.

❗ Current head a4eb444 differs from pull request most recent head 40d62c7. Consider uploading reports for the commit 40d62c7 to get more accurate results

@@           Coverage Diff           @@
##           master    #2599   +/-   ##
=======================================
  Coverage   98.27%   98.27%           
=======================================
  Files         273      273           
  Lines       10795    10795           
=======================================
  Hits        10609    10609           
  Misses        186      186

Impacted Files	Coverage Δ
...an3/alignment/scoring/aminoacid_scoring_scheme.hpp	`88.23% <100.00%> (ø)`
test/include/seqan3/test/tmp_directory.hpp	`100.00% <0.00%> (ø)`
include/seqan3/utility/views/type_reduce.hpp
include/seqan3/range/views/type_reduce.hpp	`100.00% <0.00%> (ø)`

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 cca92c4...40d62c7. Read the comment docs.

Irallia · 2021-05-05T09:56:48Z

CHANGELOG.md

+#### Alignment
+
+* The member constants of `seqan3::aminoacid_similarity_matrix` were changed to lower case
+  ([\#....](https://github.com/seqan/seqan3/pull/....)):


Suggested change

([\#....](https://github.com/seqan/seqan3/pull/....)):

([\#2599](https://github.com/seqan/seqan3/pull/2599)):

I am confused by this suggestion. Arn't you the author and should add this change?

Yes, I marked it for me to make the change when I received your review. Sorry for the confusion.

SGSSGene · 2021-05-07T07:54:43Z

CHANGELOG.md

+#### Alignment
+
+* The member constants of `seqan3::aminoacid_similarity_matrix` were changed to lower case
+  ([\#....](https://github.com/seqan/seqan3/pull/....)):


I am confused by this suggestion. Arn't you the author and should add this change?

SGSSGene · 2021-05-07T07:55:36Z

include/seqan3/alignment/scoring/aminoacid_scoring_scheme.hpp

@@ -28,33 +28,39 @@ namespace seqan3
 *
 * This enum provides IDs for amino acid similarity matrixes of the
 * [BLOSUM](https://en.wikipedia.org/wiki/BLOSUM) and [PAM](https://en.wikipedia.org/wiki/Point_accepted_mutation)
- * families.
+ * families (last access 04.05.2021).


I like this!

I'm not a big fan of this (in this context, having a bibliography is something else :)):

No one will ever update the dates even if they check the links

We don't use any information from the links, it's just some \seealso basically

Access dates are implicitly stored in the git history

Agree with @eseiler; if we can somehow automatically add that date I would love that, but manual management isn't feasible.

I'm okay with undoing it. I just want to point out that I had discussed and started this in another PR with @smehringer and @MitraDarja.
-> #2290 (comment)

Why do you think you do not update this date when you edit the documentation for it?
And wouldn't an automatic check be error-prone? Then one checks only whether the page is there, but not whether it still contains what we have taken over or to which we want to refer?

EDIT: or do you especially mean this link?

The "last access" only has to be updated if the enum provided IDs on wikipedia change. Isn't this very helpful? One could just look at the wikipedia diff from 04.05.2021 until today and see if anything has changed.

The "last access" only has to be updated if the enum provided IDs on wikipedia change. Isn't this very helpful? One could just look at the wikipedia diff from 04.05.2021 until today and see if anything has changed.

But the IDs come from some paper, it's not an invention of Wikipedia. The papers won't change and to check for some change would require you to manually look into it. I don't see this happen.

If it's not automatic, it's not reliable (include order, first brief or author, | at end or beginning of lines, how are lambdas indented, are the used URLs at all valid?).

Why do you think you do not update this date when you edit the documentation for it?

Easy to overlook if I don't even touch text in vicinity of the URL; then you would rely on someone else to know that there are links and verify them. And we have quite a few of them....

It would be nice to have, but it is also something that is quite useless if not carefully curated.

I understand the objection, but I would trust us that one gets into the habit of looking for such links. But I have no strong feelings and have undone it.

So if I understand correctly, your objection is actually against citing Wikipedia and rather cite the original paper?

test/api_stability/3.0.2/0002-NOAPI-DETAIL-MISC-Remove-top-level-scoring-scheme-de.patch

marehr · 2021-05-10T10:06:55Z

test/api_stability/3.0.2/0002-NOAPI-DETAIL-MISC-Remove-top-level-scoring-scheme-de.patch

@@ -20,7 +20,7 @@ index ef141a7e9..7b0927215 100644
 -                                                                seqan3::align_cfg::method_global,


please don't change patches, if they don't work they need to be changed differently :)

marehr

nearly done, api stability patches shouldn't be changed :)

Signed-off-by: Lydia Buntrock <[email protected]>

marehr requested review from a team and SGSSGene and removed request for a team May 4, 2021 15:14

Irallia commented May 5, 2021

View reviewed changes

SGSSGene requested changes May 7, 2021

View reviewed changes

Irallia force-pushed the misc/alignment/aminoacid_scoring_scheme_lower_case_member_constants branch from 604e198 to a4eb444 Compare May 7, 2021 10:08

Irallia requested a review from SGSSGene May 7, 2021 11:07

marehr mentioned this pull request May 10, 2021

[INFRA] last accessed via links seqan/product_backlog#385

Open

2 tasks

SGSSGene approved these changes May 10, 2021

View reviewed changes

Irallia requested review from a team and marehr and removed request for a team May 10, 2021 10:03

marehr reviewed May 10, 2021

View reviewed changes

marehr and others added 3 commits May 10, 2021 12:38

[MISC] aminoacid_scoring_scheme: lower case member constants

56b9eff

Signed-off-by: Lydia Buntrock <[email protected]>

[MISC] Deprecate upper case

95e11da

Signed-off-by: Lydia Buntrock <[email protected]>

[DOC] Add changelog entry

40d62c7

Signed-off-by: Lydia Buntrock <[email protected]>

Irallia force-pushed the misc/alignment/aminoacid_scoring_scheme_lower_case_member_constants branch from a4eb444 to 40d62c7 Compare May 10, 2021 10:39

marehr self-requested a review May 10, 2021 11:30

marehr approved these changes May 10, 2021

View reviewed changes

marehr merged commit f891e79 into seqan:master May 10, 2021

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[MISC] Alignment: aminoacid scoring scheme - lower case member constants #2599

[MISC] Alignment: aminoacid scoring scheme - lower case member constants #2599

Irallia commented May 4, 2021

vercel bot commented May 4, 2021 •

edited

Loading

codecov bot commented May 4, 2021 •

edited

Loading

Irallia May 5, 2021

SGSSGene May 7, 2021

Irallia May 7, 2021

SGSSGene May 7, 2021

SGSSGene May 7, 2021

eseiler May 7, 2021

marehr May 7, 2021

Irallia May 7, 2021 •

edited

Loading

SGSSGene May 7, 2021

eseiler May 7, 2021 •

edited

Loading

Irallia May 7, 2021

SGSSGene May 7, 2021

marehr May 10, 2021

marehr left a comment

	([\#....](https://github.com/seqan/seqan3/pull/....)):
	([\#2599](https://github.com/seqan/seqan3/pull/2599)):

		@@ -20,7 +20,7 @@ index ef141a7e9..7b0927215 100644
		- seqan3::align_cfg::method_global,

[MISC] Alignment: aminoacid scoring scheme - lower case member constants #2599

[MISC] Alignment: aminoacid scoring scheme - lower case member constants #2599

Conversation

Irallia commented May 4, 2021

vercel bot commented May 4, 2021 • edited Loading

codecov bot commented May 4, 2021 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Irallia May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

eseiler May 7, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

marehr left a comment

Choose a reason for hiding this comment

vercel bot commented May 4, 2021 •

edited

Loading

codecov bot commented May 4, 2021 •

edited

Loading

Irallia May 7, 2021 •

edited

Loading

eseiler May 7, 2021 •

edited

Loading