Repository navigation
[Exercise Approaches]: Audit/Cleanup of Track Approaches 2026 (part 2) #4197
Description
Activity
That markdown table is ... terrifying. We might need to try something else. I will think on it. 😄
Reacted by YrahcazJust went and updated the entries for the approaches that I audited... Yeah, that table's terrifying.
Edit: Just went and normalized the formatting on the table as well. ADHD beats terror every time 😆.
Reacted by BethanyGReacted by BethanyGAddressed in PR #4258:
Acronym Dig Deeper audit
-
Overall:
- The usage of "scrub the input" is a bit odd; I would have expected "clean the input" to be used. Here, "scrub" means "remove", but the cleaning for some approaches is not just removing characters.
- There is inconsistency between single quotes and double quotes, when it should be consistent within the same exercise.
- There are hyphens that should be em-dashes.
- Some comments are right up against the
#, which can hurt readability.
-
- Line 15 has a stray backtick at the end.
- Lines 25, 26 and 31: The markdown links are not rendered because the link definitions are missing from the bottom of the file.
- Line 54: "Generator" should not be capitalized here.
- The generator expression approach has an unnecessary extra set of parentheses. (Lines 64 to 70.)
- Line 134 and 141: Are all of the approaches really idiomatic? The
re.sub()approach is called a "fun experiment" in its approach doc, it is many times slower than the others, and it can be hard to understand. - Line 149 is confusing with its two instances of "least performant". It should be rewritten to clarify that
re.subis the slowest by far.
-
loop/content.md, list-comprehension/content.md,generator-expression/content.md,functools-reduce/content.md, andmap-function/content.md:- The first list item is incorrect:
- Nothing is done with spaces or single quotes in the input at all.
- Saying "scrub" or "remove" is kind of misleading, as the characters are replaced with spaces, not empty strings.
- The paragraph after the list is confusing, as the returned string (versus returned list) is what the chained method is called on. It's especially misleading because
<str>.replace()has actual string arguments, and it might make it sound like somehow those turn into lists. - The last line of the
exercism/note: Is "As of this writing" a valid way to say "when this was written"? The phrase reads kind of strangely.
- The first list item is incorrect:
-
- Line 31: There is a stray parenthesis:
'GNU'[0], 'IMAGE'[0], 'MANIPULATION'[0]), 'PROGRAM'[0] - Line 37: "acronym rather" should be "acronym, rather".
- Line 31: There is a stray parenthesis:
-
- Line 40: The empty string should be surrounded by backticks for readability and clarity.
- Line 40: "ahead/before" is rather confusing. Shouldn't it just be "before"?
-
regex-join/content.mdandregex-sub/content.md:- The first line of prose rather confusing.
-
- Line 56: "omit matching on" is confusing, it should probably be "stop matching on".
- Lines 56 and 59: The space surrounded by backticks does not render properly.
- Line 82: "generator expression + join + upper" should be changed to be more readable.
-
- Line 11 and 16:
returnandrehave an extra space between them. - Line 31 says "the phrase" where everywhere else it is referred to as
to_abbreviate. - Line 50 is missing numbers in the "this will match ..." From the docs:
For Unicode (str) patterns:
Matches Unicode word characters; this includes all Unicode alphanumeric characters (as defined by str.isalnum()), as well as the underscore (_).
Matches [a-zA-Z0-9_] if the ASCII flag is used.
For 8-bit (bytes) patterns:
Matches characters considered alphanumeric in the ASCII character set; this is equivalent to [a-zA-Z0-9_]. If the LOCALE flag is used, matches characters considered alphanumeric in the current locale and the underscore.
- Line 59: The space surrounded by backticks does not render properly.
- Line 59: Both
-should be_. - Line 59: The empty string should be surrounded by backticks for readability and clarity.
- Line 11 and 16:
-
The performance article:
- Line 5 says "seven idiomatic approaches" but then the list lays out eight different approaches.
- Line 17 says a ninth will be added, but its description of it is basically the same as number 7 in the list.
- Line 22:
numpyis callednumbpy. - Line 25 is strange, adding a word like "however" at the beginning or the end would fix that.
- Line 27: "10x or more times" should be "10 (or more) times".
- Line 29: "easiest read" should be "easiest to read".
- The
re.findall()approach always has "first letters" by it, but it is not clear that this means a differentre.findall()approach than what is in the approach doc.
@BethanyG, I have a question: My solution seems to use an approach that doesn't have an approach doc. Is it worth adding a new approach doc for this? I'd probably need some guidance on how to update and use the benchmark application.
-
@BethanyG, I have a question: My solution seems to use an approach that doesn't have an approach doc. Is it worth adding a new approach doc for this? I'd probably need some guidance on how to update and use the benchmark application.
Sure! why not?. I say go for it, and I am happy to help you use/update the benchmarking! 😄
Reacted by YrahcazAddressed in PR #4226:
Reverse String Dig Deeper audit (part 1 of 2)
-
Overall:
- There appears to be mixed quote types; I think these are supposed to be consistent per exercise.
-
- Line 17: "Range()" should be "
range()". - Line 26: "non ASCII" should be "non-ASCII".
- Line 39 is missing a period at the end.
- Line 55: "But" should probably be "However,"
- Line 67:
#Forshould probably be replaced with# <-- For. - Many terms are missing backticks around them. (Lines 17-20, 55, 60, 72, 80, etc.)
- Line 17: "Range()" should be "
-
- Line 24 seems to use
**for bold, but the website renders it as italics instead? - It saying "the space before" is very confusing, as there are actual spaces that were put in the syntax definition (probably for readability). It should either refer to the placeholders in a different way or the spaces in the syntax thing should be removed.
- Line 26: Both "-->"s should be replaced with "to"s.
- The paragraph on lines 24 to 30 is very long, and it jumps around to multiple different sub-topics, so it should be split up.
- Line 33: The hyphen should be an em-dash.
- Line 37:
O(n + n)is not valid Big-O notation, it should be changed toO(n)or just removed entirely.
- Line 24 seems to use
-
iteration-and-concatenation/content.md:- Lines 16, 17, and 19: 'output' should probably be
output. - Line 17 vs 18: One has
O(m + n)and the other hasO(n + m), it should be consistent. - Line 22: There should be an "it" between "that" and "is".
- Line 22: "O(n**2)" should be surrounded with backticks.
- Line 65: There should be a space in between the
=and''. - Line 77: "tine" should be "time".
- Lines 35 and 84: Since the
+operator is actually used in this approach, it would be clearer if usages of "+" in the prose were replaced with "and" or "plus".
- Lines 16, 17, and 19: 'output' should probably be
-
backward-iteration-with-range/content.md:- Line 7:
#Forshould probably be replaced with# <-- For. - Lines 13 and 34: "-->" should be replaced with "to".
- Line 18 is a bit confusing, adding parentheses around the "mentioned in the iteration and concatenation approach" would help.
- Lines 34 and 35 seem to be wrong. This variant is still appending, not prepending.
- Line 51: There should be an "it" in between "but" and "becomes".
- Line 52: "is not strictly repetition and length constrained" is very confusing, I'm not quite sure what it's supposed to mean.
- Line 7:
-
Addressed in PR #4226:
Reverse String Dig Deeper audit (part 2 of 2)
-
- Line 80: The period should probably be removed, as it is a header, not a sentence.
- Line 88: There should probably be a space in between the
,andcodepoint. - Lines 99, 113, 116, 127, etc.: Terms missing backticks.
- Line 146: "and or" should be "and/or".
- Line 146 is missing a period at the end.
- Line 159: "intput" should be "input".
-
- Line 6: The hyphen should probably be replaced with "to" or "or".
- Line 14: There should probably be a space in between the
,andcodepoint. - Line 19: "O(1)" should be surrounded with backticks.
- Line 24: "Range" should be "
range()". - Line 24: "list.append()" should be surrounded with backticks.
- Line 42: "range()" should be surrounded with backticks.
- Line 42: "over 1/2" should probably be "Over Half".
- Lines 48, 52, and 74: There is no space in between the
#and the comment. - Line 58: The usage of "a median" is confusing here, maybe use "the midpoint" (or similar) instead?
- Line 61 is a bit confusing, maybe "each force iteration over" should be "both iterate over".
- Line 82: More variables missing backticks.
- Line 104: There is an extra space in between "an" and "
UnicodeDecodeError". Also, should this "an" be an "a"? It reads more naturally with an "a", but I'm not sure which is correct.
-
built-in-list-reverse/content.md:- Title: The period should probably be removed, as it is a title/header, not a sentence.
- Line 13: "loop + append" should probably be "loop and
list.append()" for clarity. - Line 19: There should be a comma in between "
None" and "as". - Line 47 has three spaces of indentation instead of four.
-
- Title: The period should probably be removed, as it is a title/header, not a sentence.
- The variant being in
introduction.md(and not having a header of its own inbuilt-in-reversed/content.md) is confusing, especially because it does not usestr.join(), which is in the header for the approach inintroduction.md. - The first code block has unnecessary parentheses.
-
additional-approaches/content.md:- Line 26: There should be a space in between the
+and1. - Some comments are missing a space after the
#. - Line 31 would be clearer if the "or" was a "/" instead.
- Many terms are missing backticks.
- Line 43: "the code below" should be "the next approach" for clarity.
- What is
stniopedocsupposed to mean? It doesn't seem like a good variable name. - Line 65: "into a the" should be "into the".
- Line 70: "almost" should be "slightly", as "almost slower" implies that it is slightly faster.
- Line 88: "slice + concatenation" should be "slice plus concatenation" for clarity.
- Line 90: Wouldn't it be
O(n**2)space complexity (wherenis the length of the string)? - Line 95:
lambbdashould belambda. - Line 95:
Join()should bejoin(). - Lines 99 and 110: The parentheses around
-x-1are unnecessary. - Line 106: "to
O(n)" should be removed, it was alreadyO(n)(even though it was less efficient). - Line 127: There should be a comma in between "issues" and "since".
- Line 129: There should be an "it" in between "but" and "does".
- Line 26: There should be a space in between the
-
The performance article:
- The snippet appears to be broken, it seems like both Github and the Exercism website fail to render the Markdown table properly.
- Line 5: "six" should be "seven".
- Line 15: "and a seventh approach using
map()" should be "(for the 'other interesting approaches' category, themap()approach will be used)"
@BethanyG, Can I open a PR to fix these?
-
@Yrahcaz7 - apologies. 😅 I was out today running errands and food shopping, so didn't get to this until now. Since I wrote this set of approaches, I sorta feel obligated to correct them. But if you really want to PR them, have at it. 😄 💙
Reacted by Yrahcaz2 remaining items
Edit: I get a 404 error when I try to access the ternary operator approach, even though the entry is present in the config.json and the files are in the correctly named folder on GitHub.
@BethanyG, do you know what's going on here?
🤔 I just took a quick look. I don't know what is up. There doesn't appear to be a mis-match in folder name, name in config, or anything in the reflink. One thing I can think of that might be an issue is that the UUID for the article might not be unique, or there might be something poisonous in the formatting of the file. That author was fairly famous for cutting and pasting from other tracks and not changing the UUIDs ... so.
Maybe we try re-creating that approach by re-doing the UUID for the article, and pulling out the text of the approach and re-saving it? I can do that after I finish reviewing your Eilud's Eggs approaches. 😄
Reacted by YrahcazAddressed in PR 4252:
Leap Dig Deeper audit (part 2 of 2)
-
- Line 15: In "
datetimeobject and adatetime.timedeltaobject", either "datetimeobject" should be "datetime.datetimeobject", or "datetime.timedeltaobject" should be "timedeltaobject". As it is now, it is a bit confusing and inconsistent. - Line 17: "exercises" should be "exercise's" here.
- Line 20: The second comma isn't necessary here.
- Lines 23 to 25: This bulleted list should be a paragraph instead of a list. It is also kind of confusing. Really, all of lines 20 to 25 should probably be merged into one unified explanation that is clearer.
- Some terms are missing backticks.
- Line 15: In "
-
- Line 3: The codeblock does not have syntax highlighting because the language is set to "pythoon" instead of "python".
- Line 24/37 should probably use the link from
introduction.md, as that one goes directly to the implementation instead of just the top of the file.
-
The performance article:
- Lines 11 to 12: The list does not render properly because there is not a blank line in between the list and the previous paragraph.
- Line 22: The chart's alt-text has the wrong dates: "1900, 200, 2019, and 202" should be "1900, 2000, 2019, and 2020".
- Line 22: The mentioned heading "Timings for approaches by input year" should probably be surrounded by quotes instead of the whole alt-text.
- Lines 38 to 45: These list items should probably say "approach", as in "The ternary operator approach" instead of just "The ternary operator".
- Line 42: "but it is" should probably be "it is also".
- Line 45: "then" doesn't seem necessary here.
Reacted by BethanyG-
with any luck... the Leap PR will fix the missing approach.
It seems like it's fixed now!
Reacted by BethanyGReacted by BethanyGRegarding the "Acronym" approach, instead of using "scrub the input," it might be clearer to say "clean the input" to align with standard terminology in data processing. Additionally, can we clarify what the expected format for the input string is in this approach to enhance clarity for users?
@Fi6uDvGWucFv - the changes to the
Acronymexercise have already been PR-d.@BethanyG, actually, I think we never PRed those changes. I still see the old version on the website -- We probably both confused the new approach with the cleanup and never actually PRed it 😅.
Should I draft up a PR? I'll probably have the time to do so tomorrow.
@BethanyG, actually, I think we never PRed those changes. I still see the old version on the website -- We probably both confused the new approach with the cleanup and never actually PRed it 😅.
Should I draft up a PR? I'll probably have the time to do so tomorrow.Sure! Let's go ahead and do that. 😄 Thanks!
Bob Dig Deeper audit
-
- Line 6 is confusing because it isn't clear that it is referring to a different approach. Making it a markdown list of the approaches would be one way of fixing this.
- Line 10 should probably have a colon at the end.
- Line 22 (line 25 in
if-statements/content.mdandif-statements-nested): "If anifstatement can return" is confusing, as it makes it seem like theifstatement only sometimes returns. It should probably be "If anifstatement returns" instead. - The first two approaches don't have any explanation in
introduction.md. Should this be added? - Line 71: The header "Other approach: answer list" should probably be "Approach: Answer list" because it is already under the "other approaches" header.
- The last approach is missing a codeblock; was this intentional?
- Line 83 is formatted as a list because of the "
-" at the beginning. - Line 86: This link has the wrong
highlightparameter. The parameter should either be removed or changed toendswith: https://docs.python.org/3/library/stdtypes.html?highlight=endswith#str.endswith - Overall: "
ifstatements nested" is a bit awkward, "nestedifstatements" would be clearer.
-
if-statements/content.mdandif-statements-nested/content.md:- Line 38 (line 43 in
answer-list/content.md): "Z" should be "Z". Also, "when uppercase" should probably be "when uppercased" for grammar.
- Line 38 (line 43 in
-
if-statements-nested/content.md:- Line 44: This is false because it is just an
ifstatement, not anif/else. - Line 44: "returns if it is a shouted question or just a shout" is confusing, as Bob's response is what is returned, not the type of the input sentence. Replacing "returns" with "tests" would make it clearer.
- Line 44: This is false because it is just an
-
- Line 26: This link should be converted to a reflink. The linked
python-tuplesheader also seems to have been renamed tocreating-tuples-in-python. - Line 27 is rather confusing. "the performance between a tuple and a list here is equivalent" should probably be changed to "a tuple does not perform differently than a list here" or similar.
- Lines 27-28: There is no transition between these two lines, which is a little jarring. Line 28 should probably start with "However," or similar.
- Lines 34-36: There is no transition between these two lines. Line 36 should probably start with "Next," or similar.
- Line 52 should have a colon at the end.
- The snippet has
# code snippedinstead of the usual...; which one of these is preferred?
- Line 26: This link should be converted to a reflink. The linked
-
The performance article:
- Line 3: "approach" should be "article".
- Line 26: This link redirects to: https://exercism.org/tracks/python/exercises/bob/dig_deeper
-
For the Affine Cipher exercise, consider including a function that explicitly demonstrates how the encoding and decoding are related mathematically, which would enhance clarity and educational value for new learners.
@kyliekamesha26 - I'm sorry, but this issue is closed to contributions from anyone but Yrahcaz7, who was originally assigned to it.
Roman Numerals Dig Deeper audit
-
Overall:
- "Loop over Romans" does not make sense, it should be changed to "Loop over Roman Numerals".
-
- Line 20: "concept" should probably be "process" or similar.
- Lines 20-24: This paragraph should be made more generic; right now it seems to focus too much on the "With
ifconditions" approach. - Lines 24 and 95 should probably be separated from the last list item.
- Line 91 is missing punctuation at the end.
- Line 92 is a bit awkward.
- Line 122: This is an odd place for the link, perhaps it should be moved to line 133.
- Lines 135-147 should probably be reworded to use a link to the relevant lines instead of a codeblock.
- Lines 175-180: This codeblock is formatted in a way that is very hard to read.
- Line 179 uses double quotes.
- The introduction is missing a section that links to the
itertools.starmap()approach.
-
if-else/content.md,if-else/snippet.txt, andintroduction.md:- This approach uses type hints, which are usually not included. It also has unnecessary assertions.
-
- Lines 17, 20, 22, 24, 26, and 35: There is an extra space after the
+=. - Lines 28 and 37: There is an extra space after the second
>. - The first codeblock has unnecessary
> 0and> 4comparisons. - Line 49: This paragraph does not actually explain how the approach works.
- Line 61 needs more explanation.
- Line 64 is missing a serial comma in "hundreds, tens and units".
- Line 69 does not make much sense where it is currently, so it should be moved to after the codeblock.
- Lines 17, 20, 22, 24, 26, and 35: There is an extra space after the
-
table-lookup/content.md,table-lookup/snippet.txt, andintroduction.md:- This approach uses single letter variable names (that are not roman numerals). It also has an unnecessary assertion.
-
- Perhaps the comments could be merged into the rest of the explanation.
- Line 17 (75 in
introduction.md): "we need" should be "get" and "our" should be "the". - Line 34: This header should be "Variation 1" or similar.
-
- This snippet uses single letter variable names (that are not roman numerals).
- Line 2 (line 30 in
loop-over-romans/content.md) has an unnecessary assertion. - Line 3 uses double quotes (quoting should be consistent per exercise, if I remember correctly).
-
- This approach uses type hints.
- The exact workings of most of the variants are not explained.
- There are no headers separating the variants.
- Line 19: "Arabic (decimal)" should be "Arabic (decimal) numbers".
- Lines 25-27, 70, and 89 use double quotes.
- Lines 33-36 and 55-57 use single letter variable names (that are not roman numerals).
- Line 40: "the" is unnecessary here.
- Lines 44 and 45 have inconsistent spacing.
- Lines 48 and 74 have docstrings.
- Line 66: "1, 5, 10" should be "1, 5, and 10".
- Line 69 is missing a serial comma in "10's, 5's and 1's".
- Line 97: "is doing" should probably be "does".
- Line 113 does not apply to the last variant. This should be clarified.
- Line 113: There is a missing space after "
while". - Line 117 is a bit awkward, changing "so need" to "so they need" should improve it.
- Lines 121-124 should probably either be:
- removed entirely, or
- moved to a single variant (and preserve the type hints on only that variant) and also have links to resources about type hinting.
- If it is kept, these should be fixed:
- Line 122: There should be an "is" after the "and".
- Line 124: "IDE's" should be "IDEs". Also, the subject of whether VSCode is an IDE should probably be avoided.
-
recurse-match/content.md,recurse-match/snippet.txt, andintroduction.md:- This approach uses type hints.
- The comma in
[ROMAN_NUM[idx],]is unnecessary, as it is a single-element list, not a tuple. - The last case (
case [num, idx, digits]:) can be changed to a default case (case _:).
-
- Lines 5, 38, 39, and 50 (line 155 in
introduction.md) use double quotes. - Line 22: The first part of this sentence could be improved to: "A major limitation is the lack of tail-call optimization".
- Line 26 has an extra space before the ending period.
- Line 41 has an extra space after
convert. - Lines 44-46: It is not clear what the
ltrvariable represents from its name. It should probably be renamed toroman_valor similar. - Line 57: This link is invalid as it uses
loop-over-romaninstead ofloop-over-romans.
- Lines 5, 38, 39, and 50 (line 155 in
-
itertools-starmap/content.mdanditertools-starmap/snippet.txt:- This approach uses type hints, single letter variable names, and double quotes.
-
- Line 50 is a bit awkward; perhaps it could be changed to: "Roman numerals for 5,000 and 10,000 are not defined, so spaces are used here instead."
- Line 63: There should be a comma after "For example".
- Line 71: "
compute" should probably be "compute()".
-
- Line 23: "Numerals" should be lowercased here.
- Line 53: Maybe "the recently-introduced" should be removed, as Python pattern matching has been around for almost 5 years now.
@BethanyG, can I open a PR to fix these? (Or perhaps 2 separate ones due to the amount?)
-
Hi @Yrahcaz7 👋🏽
@BethanyG, can I open a PR to fix these? (Or perhaps 2 separate ones due to the amount?)
Apologies for the delay in replying. Go for it. Maybe two groups: The straight-up typos, whitespace, and punctuation-related stuff. And a second one for rephrases and other more substantial edits? Many thanks for working on this!
Reacted by YrahcazI can take a look at auditing
Circular Buffernext, specifically ensuring we cover the trade-offs between using a
This is a tracking issue for existing approaches on the Python track.
(Part 1 of the audit/cleanup was issue #4055.)
In general, we are auditing/reviewing the following:
See the lists for each category below. Exercises in "Merged Audits" have been reviewed, changed, and merged. Exercises in "Unaudited" have approaches that have not been audited yet.
Exercises in "Approaches In Progress" do not have finished/published approaches yet, while those in "Missing Approaches" do not have any known work on their approaches yet.
If you want to work on an exercise that is missing an approach, please check in with the maintainers to avoid conflicting with current track priorities or others who may be working on things.
Merged Audits
ALPHABETcapitalization and approaches link #4184Unaudited
Approaches In Progress
Missing Approaches