Python icon indicating copy to clipboard operation
Python copied to clipboard

Fixes wiki and doctest issues

Open MarlieChiller opened this issue 2 years ago • 6 comments

Describe your change:

Fixes a markdown typo in the wiki (unable to edit the wiki directly in a PR so I think @Siddharth1605 added the files as new to be ported). In doing so, the ci doctests pipeline failed for calculate_age in web_programming/get_top_billionaires.py which I refactored and addressed

[closes #11225]

  • [ ] Add an algorithm?
  • [ ] Fix a bug or typo in an existing algorithm?
  • [x] Add or change doctests? -- Note: Please avoid changing both code and tests in a single pull request.
  • [x] Documentation change?

Checklist:

  • [x] I have read CONTRIBUTING.md.
  • [x] This pull request is all my own work -- I have not plagiarized.
  • [x] I know that pull requests will not be merged if they fail the automated tests.
  • [x] This PR only changes one algorithm file. To ease review, please open separate PRs for separate algorithms.
  • [ ] All new Python files are placed inside an existing directory.
  • [ ] All filenames are in all lowercase characters with no spaces or dashes.
  • [ ] All functions and variable names follow Python naming conventions.
  • [ ] All function parameters and return values are annotated with Python type hints.
  • [x] All functions have doctests that pass the automated testing.
  • [ ] All new algorithms include at least one URL that points to Wikipedia or another similar explanation.
  • [x] If this pull request resolves one or more open issues then the description above includes the issue number(s) with a closing keyword: "Fixes #ISSUE-NUMBER".

MarlieChiller avatar Jan 02 '24 10:01 MarlieChiller

I havent taken the time to fully understand the code in the calculate_age function but the doctests seem a bit brittle here. It doesnt seem coincidence that these fail just after a new year starts. A little strange that its a smaller number rather than a larger number as well but that could be down to my limited undestanding

MarlieChiller avatar Jan 02 '24 10:01 MarlieChiller

@cclauss Ive updated the whole calculate_age function and related doctests as I dont think it was achieving what it set out to do initially. Please review changes when appropriate. I can see new PRs against this repo are now failing due to this function and its doctests in master branch

MarlieChiller avatar Jan 03 '24 11:01 MarlieChiller

The get_top_billionaires() issue was fixed in https://github.com/TheAlgorithms/Python/pull/11234 so please revert all changes to that file.

cclauss avatar Jan 15 '24 09:01 cclauss

I am lost trying to figure out how this works. If I go to https://github.com/TheAlgorithms/Python/wiki I already get most of this. Could you let me know why what we have already is not enough?

When I click on List of Algorithms and scroll down to Sorts I get https://github.com/TheAlgorithms/Python/blob/master/DIRECTORY.md#sorts which is an alphabetized list of all sort algorithms that is kept up to date by https://github.com/TheAlgorithms/Python/blob/master/.github/workflows/directory_writer.yml every time a pull request is created. Why do we want a manual, unsorted list of sorts? How is that a better user experience?

It is more customary and easier to find if the information about best case, worst case, is documented in https://github.com/TheAlgorithms/Python/blob/master/sorts/README.md

cclauss avatar Jan 15 '24 14:01 cclauss

This PR is literally only to fix the typos that I found in the markdown of the wiki as mentioned in 11225. You only need to correct those typos now as you have fixed the other changes that I spent time correcting in get_top_billionaires.py.

MarlieChiller avatar Jan 15 '24 18:01 MarlieChiller

To put it most simply, please visit https://github.com/TheAlgorithms/Python/wiki/Sorting-Algorithms#selection-sort. It is misformatted and links to the wrong algorithm (bubble sort rather than selection sort). You can lift the relevant markdown from this PR directly into the wiki to resolve this issue. Github has no way of easily opening PRs against wiki pages

MarlieChiller avatar Jan 15 '24 18:01 MarlieChiller