Docstring Cleanup #3007
Docstring Cleanup #3007
Conversation
So that they're consistent with proper function spacing of 2 lines
Also included newlines after d-strings for constency in stub file. Removed extra white-spaces: lines 5, 15, and 36 - both stub & exemplar.
Added summary first line docstrings to both stub and exemplar files
Consistent with discussed docstring style format Removed whitespace on line 8 & 49 in stub and exemplar
Used the Union type to represent that the type can be either int/float, per this documentation: https://docs.python.org/3/library/typing.html#typing.Union Added period to top level docstring (per pep257) Removed whitespace on line 1 (pep257) and 9 Changed 'boolean' to 'bool' on line 9 exemplar was missing top level (module) docstring. added newlines after each exemplar.py multiline docstrings
Also added dashes as per discussions on style Began adding periods to the end of return and param descriptions, as per discussion in issue this branch is based on brought up by Isaac. Will go back and add this to others and make note of other exercises that have this inconsistency removed extra whitespace on lines 5, 15, & 37 added the word 'return' on line 18 so that there is a lowercase letter at the starting word of the description
Missing periods at the end of param and return lines
added descriptions to all return lines in docstrings
added periods to param and return lines in all docstrings line 9 changed to begin description /w lowercase letter (stub & exemp)
return line in 'higher_card' docstring formatted to PEP257 standard
'higher_card' docstring return line description to start as lowercase
added a space on line 15 in exemplar (Flake8 E225)
suggestion for type in 'higher_card', {str or (str, str)}
re-wrote the module level docstring to be more descriptive added periods to 'add_prefix_un' & 'adjective_to_verb' param & return line in docstrings
added module level docstring in stub and exemplar (both files) added summary first line docstrings (both) cleaned up extra whitespaces, such as lines 8, 9 in stub added newlines after docstrings per repository standard some indentation issues in exemplar docstrings
whitespace in docstrings on lines 33, 43 in stub & exemplar
added module level docstring in stub and exemplar (both files). various whitespace and tabbing issues cleaned up (both). added types to all docstring return and param lines (both). added some periods to docstring descriptions. brought the example portion of letter_grades docstring to exemplar
added module level docstrings to both stub and exemplar (both files). added summary first line docstrings (both). converted the 'tuple' types to tuple syntax with nested types in param and return lines in docstrings (not attached, its consistent with other concepts were altered though) added periods at the end of some descriptions (both). removed some extra whitespaces (both) added a newline on line 42 in exemplar grammar fix on line 50 in stub, line 55 in exemplar changed the return line of 'compare_records' docstring to start with lowercase letter (both)
added module level docstring to both stub and exmplar (both files). added summary first line docstrings to each function (both). removed whitespace on lines 8, 19, 30, 41 in stub removed whitespace on lines 8, 21, 35, 49 in exemplar.
added a module level docstring to both stub and exemplar (both files) added summary first line docstrings to all (both). added dashes and periods to return/param line descriptions. (both) added missing param and return line descriptions to docstrings (both) cleaned up the 'intersection' param docstrings -- the long list of constant var names was moved to the additional notes section (both) removed various extra whitespaces (both) 'singleton_ingredients' dishes & intersection params swapped places (both) newline on line 104 in stub newline on line 124 in exemplar
per title, tuples as types looked very messy so they were reverted to just "tuples" in docstring types added hints in regards to how to format the multiline strings (clean_up)
felt it was more succinct to just use 'number' as type since integer or float is given in the description, also looked messier.
on line 22 in stub and line 31 in exemplar
'str' after the dash, line 17 in stub and exemplar. made the module level docstring more detailed.
per title, reverting the list type changes so docstrings are easier to read in both stub and exemplar (both) missing types and dashes in `perfect_score` (both)
This comment was marked as resolved.
This comment was marked as resolved.
There was a problem hiding this comment.
oof. @Metallifax -- first, thank you so much for all your work on this. 🌟 💙 💛 🌟
I know it was a lot, and I think I may have just upped the effort by making so many comments - so I sincerely apologize for that. Overall, I think the changes are good -- but in many cases, I thought the leading sentence of the docstring was too much. If we are going to the effort to list out the inputs and return types on their own lines, it seems overkill to then re-iterate the same information in the lead-in sentence. It feels like it should be one or the other -- but not both.
Apologies for places where I did not make the same suggestions in the examplar.py that I did in the stub, or vice-versa -- please let me know if you need me to go back and do that.
Also -- many of these are suggestions and based on my personal preference, so if you disagree, you can ignore them. I have gone over these files a few too many times, and have lost the "eyes" of someone who is trying to solve the exercise -- so I may not be the best judge of what is or is not confusing at this point in time.
I'll follow up with a comment on the topics you listed in your PR description. I'll then hold off on anything else until you ignore/accept all these comments. I know this many is quite noisy,.
Let me know if you have follow-on questions or issues, or need help with anything.
|
@BethanyG I do find the whole "commit suggestions" thing to be tough since changes are to be made in both stub and exemplar files... Should I then just take note of our discussions and changes during review (in a Google Spreadsheet or something) and do one big commit using my local setup? Or do the same yet commit each concept separately for cleanliness' sake? I'm open to suggestions since we're in many different concepts at once and I'd very much like to get things right the first time due to changes being mirrored in both files. Thanks again for your time. |
|
Ok. Let me see if I can be coherent on your questions. 😁 Probably not, but here goes. 😆
I agree that less verbose is better but I don't like the brackets. They're too close to the format used by type aliases and type hinting and might cause even more confusion - especially when you have
I am not attached to it at all. Originally, we didn't have the cards listed, since the values are described in the instructions. I think at some point (I would have to check now), someone submitted a PR to clarify things. My hot take? We put a list in here the way we did with the criticality intervals on :param card_one, card_two: str - cards dealt in hand. See below for card values.
- 'J', 'Q', or 'K' (otherwise known as "face cards") = 10
- 'A' (ace card) = 1
- '2' - '10' = numerical value.
Any formatting we put in will not read as formatting on the site, since the editor we use doesn't support that (and these files are considered text), so I am (as you can probably tell from the mix-and-match in the docstrings themselves) a bit ambivalent. Here is a quick screengrab of part of
Note that the That being said, it would be nice to have the files appear nicely formatted to anyone who downloads them via CLI & has editor popups. So.... humm.... I think it's a balance between what would look nice and what would be confusing if read raw. Maybe if I saw some examples? |
Yup. I blew it on that one. Let me think..... Maybe it would be better if we had a more interactive discussion, and you could make changes on your setup as we went along? How would you feel about hopping on a quick Jitsi Meet session or a ... Gitter room to go through the files? I have some time this evening (I am in PDT), and might have time Friday as well. That might be smoother than going back and forth in comments over such a large set. Conversely, I can go back and make sure each stub suggestion set matches the exemplar suggestion set. I don't mind. 😄 It will take me a bit, but I am a big girl, so can take my lumps. 😆 |
|
@Metallifax -- and just to be clear -- I am perfectly fine with any of the three options. Whatever works best for you, since you went to all the work to fix these. So don't fee bad if your answer is "make all of them match, please". 😄 |
|
@BethanyG Time wise, I'm actually dangerously close to a school deadline tonight on an assignment but wanted to add to the discussion before I left. I like the idea of real time communication instead of posting things out of sync, what time were you thinking Friday? I'm Atlantic Standard Time. Also, trust me, I found my groove doing these so don't feel too too bad! |
Go. Please. 😄 Do not let me be the reason you pass a deadline. Any time after our general exercism call on Friday should work for me. It usually starts around 4:30 UTC (or at least that was the time last meeting), and runs for an hour/hour and a half. All community members (that means you, if you are interested!!) welcome. 😄 So any time after 11 am PDT Friday, which I think is 14:00/2pm AST Works for me. We could check in with each other on the exercism gitter channel, or you can ping here. If you've signed up to be a mentor and are in our mentor Slack channel, I am available there as well as good luck and good speed on the assignment! |
|
Putting on a [on hold] label until we can go over this interactively. |
line 16 words in parens.
both files return statement simplified (both stub and exemplar).
shorted param and return descriptions and made types more intuitive shuffled the additional notes in fail_safe modified the module level docstring
Added examples to make_word_groups and and adjective_to_word functions Various grammar changes and word swaps.
Committing the caiter-waiter exemplar.py changes.
More docstring edits
…ns from code review
|
@BethanyG Looks great! Thank you for all your help! |
Adresses #2974.
A sweeping docstring cleanup effort in concept exercises that includes changes such as:
Where the <description> begins with a lowercase letter and ends in a period.
What I found while working through the concepts
typingmodule style union which tells us which types the list can hold:we have just:
we then rely on the description to talk about the type and clarify, this is most evident in Metldown Mitigation and Tisbury Treasure Hunt, however, I can recognize that this can be seen as more confusing to the student, so I would love to hear some input and discuss and maybe implement something before merging.
I was also thinking of a syntax that looks succinct and readable, like:
that way you can understand what is happening without needing a background in the
typingmodule and remove a lot of extra unnecessary descriptions.How attached are we to this? I feel it would look less messy overall, especially when you hover over the function to view the docstring in the editor popup box.
pre-formatand emphasis if you think that will make our docstrings look better in editor popups. I noticed that some have them and some don't, but am curious what others think about this or if we even care about this right now.Thanks for reading! Happy reviewing!