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

Create and populate the Maths directory #4

Merged
merged 24 commits into from
Jun 23, 2023

Conversation

dlesnoff
Copy link
Collaborator

@dlesnoff dlesnoff commented Mar 14, 2023

Add:

  • maths/ directory
  • aliquot_sum.nim
  • addition_without_arithmetic.nim
  • abs.nim
  • allocation_number.nim

These are examples adapted into Nim from TheAlgorithms/Python/maths.

  • I will rebase from master to remove Contributing Guidelines and all these other stuffs when my other PR will be merged.
    I tried to respect:
  • {.push raises: [].} pragma on top of the file
  • use of Natural(s) to limit exceptions -> might be improved by Positive type?
  • use of Slices instead of Python tuples for ranges
  • runnableExamples on top to showcase the functions of the module
  • unittest under a when isMainModule block at the end

I respected also these but these are my personal conventions:

  • I do not export by default my functions.
  • I do not put a newline at the end of my files

Copy link
Contributor

@ZoomRmc ZoomRmc left a comment

Choose a reason for hiding this comment

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

Looks splendid!

maths/aliquot_sum.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
Copy link
Contributor

@pietroppeter pietroppeter left a comment

Choose a reason for hiding this comment

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

this is good thanks! my guess is you picked the first few files in alphabetical order from python repo? ;) there are some uncommon algorithms there (and my suggestion would be to skip allocation_number). :)

maths/addition_without_arithmetic.nim Outdated Show resolved Hide resolved
maths/aliquot_sum.nim Outdated Show resolved Hide resolved
maths/allocation_number.nim Outdated Show resolved Hide resolved
@Panquesito7 Panquesito7 added the enhancement New feature or request label Mar 24, 2023
ZoomRmc
ZoomRmc previously approved these changes Apr 24, 2023
@dlesnoff
Copy link
Collaborator Author

I added empty lines at end of files as requested by @Panquesito7 in another PR.

@dlesnoff
Copy link
Collaborator Author

dlesnoff commented Jun 19, 2023

@ZoomRmc I just need one approving review to merge this old pull request.

maths/aliquot_sum.nim Outdated Show resolved Hide resolved
maths/bitwise_addition.nim Outdated Show resolved Hide resolved
Copy link
Contributor

@ZoomRmc ZoomRmc left a comment

Choose a reason for hiding this comment

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

abs.nim requires a few changes to make runnableExamples compile:

  • export all used procs
  • replace absMaxSort with the updated name signedMaxAbs
    You can also add a signedMinAbs test there, while you're editing it.

BTW, IIRC, runnableExamples are compiled as debug by default (or always?), so a regular assert works in them just fine. Nothing wrong with doAssert too, of course. Just a tip.

Copy link
Collaborator Author

@dlesnoff dlesnoff left a comment

Choose a reason for hiding this comment

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

Remove quotation marks for MD inline code (without opening my editor since I am lazy).

maths/abs.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
maths/abs.nim Outdated Show resolved Hide resolved
maths/aliquot_sum.nim Outdated Show resolved Hide resolved
@ZoomRmc
Copy link
Contributor

ZoomRmc commented Jun 19, 2023

I think the RE block in abs.nim is the last thing to fix, and then it looks ready to merge!

@dlesnoff
Copy link
Collaborator Author

nim -ax r aliquot_sum.nim made me realize there was a supplementary ValueError declaration to the exception system.
Thanks for your suggestion changes, I finally opened my editor and applied them ;)

Copy link
Contributor

@ZoomRmc ZoomRmc left a comment

Choose a reason for hiding this comment

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

Nicely done!

It was interesting to see the code evolve to conform to the newly written guidelines :)

edit: My review is not enough, perhaps because of the codeowners? Or because you requested particular reviewers, not sure.

@dlesnoff
Copy link
Collaborator Author

@pietroppeter You requested changes, that might block the PR. If you are still around, can you accept the PR, to see if that's what is blocking the PR?
@Panquesito7 I think that you dimissed the review of @ZoomRmc and that merging requires your approval. (At least, as a code owner, you can resolve the situation no matter the old requested changes from @pietroppeter ).

@dlesnoff
Copy link
Collaborator Author

Looks like it requires @Panquesito7 's approval nonetheless.

@Panquesito7
Copy link
Member

Looks like it requires @Panquesito7 's approval nonetheless.

It requires approval from someone with write+ access.
There are not many maintainers currently. If someone wants to apply: #9

@Panquesito7 Panquesito7 merged commit cf013f1 into TheAlgorithms:main Jun 23, 2023
0 of 4 checks passed
@dlesnoff
Copy link
Collaborator Author

Thanks @Panquesito7 !

dlesnoff added a commit to SatinWukerORIG/Nim that referenced this pull request Jun 27, 2023
* Add a sample of maths basic algorithms

* Update maths/abs.nim

Co-authored-by: Zoom <[email protected]>

* Use openArray in absMaxSort

Co-authored-by: Zoom <[email protected]>

* Fix seq->openArray and int->SomeInteger

* Use Positive as input instead

* Update maths/addition_without_arithmetic.nim

Co-authored-by: Pietro Peterlongo <[email protected]>

* Name allocation number

* [maths/abs] Replace maxAbsSort by signed[Min/Max]Abs

* [Aliquot Sum] Add header

* Add empty line at end of file

* Remove Allocation number since it is a non standard algorithm

* Fix titles

* Run nimpretty on the files

* Rename file and add DIRECTORY.md

* abs: Fix variable name

* Add export marker

Co-authored-by: Zoom <[email protected]>

* Add RE block in aliquot sum and improve RE

Co-authored-by: Zoom <[email protected]>

* Remove MD quotation marks for inline code

* Add export marker for RE block

* Remove extra ValueError

---------

Co-authored-by: Dimitri LESNOFF <[email protected]>
Co-authored-by: Zoom <[email protected]>
Co-authored-by: Pietro Peterlongo <[email protected]>
Co-authored-by: David Leal <[email protected]>
dlesnoff added a commit to dlesnoff/Nim-Algorithms that referenced this pull request Jul 4, 2023
* Add a sample of maths basic algorithms

* Update maths/abs.nim

Co-authored-by: Zoom <[email protected]>

* Use openArray in absMaxSort

Co-authored-by: Zoom <[email protected]>

* Fix seq->openArray and int->SomeInteger

* Use Positive as input instead

* Update maths/addition_without_arithmetic.nim

Co-authored-by: Pietro Peterlongo <[email protected]>

* Name allocation number

* [maths/abs] Replace maxAbsSort by signed[Min/Max]Abs

* [Aliquot Sum] Add header

* Add empty line at end of file

* Remove Allocation number since it is a non standard algorithm

* Fix titles

* Run nimpretty on the files

* Rename file and add DIRECTORY.md

* abs: Fix variable name

* Add export marker

Co-authored-by: Zoom <[email protected]>

* Add RE block in aliquot sum and improve RE

Co-authored-by: Zoom <[email protected]>

* Remove MD quotation marks for inline code

* Add export marker for RE block

* Remove extra ValueError

---------

Co-authored-by: Dimitri LESNOFF <[email protected]>
Co-authored-by: Zoom <[email protected]>
Co-authored-by: Pietro Peterlongo <[email protected]>
Co-authored-by: David Leal <[email protected]>
dlesnoff added a commit that referenced this pull request Sep 8, 2023
* Add a .github action to build DIRECTORY.md

* Update gh action build_directory_md

* Add ignored paths and git credentials

* chore: minor Gitpod improvements (#34)

Add Gitpod badge and prebuilds for all branches and forks

* Add binary search (#37)

* Add binary search

* Update binary search: add recursive version

* Update binary search: better description

* Update binary search: update runnableExamples

* Update binary search: improve tests, add overload for recursive func

* Update binary search: evaluate if-expression into result

* grammar

* Update binary search: improve description, add comments

* Update binary search: add missing test cases

* Update binary search: add info about pivot choice in description, rearrange it

* Update binary search: change int to Natural

* Update binary search: if block -> guard clause

* Update binary search: test cases

* Update binary search: simplify docs

* Update binary search: nimpretty

* Apply suggestions from code review

Co-authored-by: David Leal <[email protected]>

* Replace by an attempt of github pr create

* Fix branching and environment variable in the PR

Co-authored-by: David Leal <[email protected]>

* Create and populate the Maths directory (#4)

* Add a sample of maths basic algorithms

* Update maths/abs.nim

Co-authored-by: Zoom <[email protected]>

* Use openArray in absMaxSort

Co-authored-by: Zoom <[email protected]>

* Fix seq->openArray and int->SomeInteger

* Use Positive as input instead

* Update maths/addition_without_arithmetic.nim

Co-authored-by: Pietro Peterlongo <[email protected]>

* Name allocation number

* [maths/abs] Replace maxAbsSort by signed[Min/Max]Abs

* [Aliquot Sum] Add header

* Add empty line at end of file

* Remove Allocation number since it is a non standard algorithm

* Fix titles

* Run nimpretty on the files

* Rename file and add DIRECTORY.md



---------

Co-authored-by: Dimitri LESNOFF <[email protected]>
Co-authored-by: David Leal <[email protected]>

* Add arc length (#10)

* Fix linear search (#33)

* Use a dynamic allocated array (sequence) for strings

* Run nimpretty

* Add requested changes

* Fix documentation generation

* Fix test title according to changes

Co-authored-by: Satin Wuker <[email protected]>

* Update comments to warn about indexing issues

Modified a bit the suggestions of comments. Please check them before approving.

Co-authored-by: Zoom <[email protected]>

---------

Co-authored-by: Dimitri LESNOFF <[email protected]>
Co-authored-by: Zoom <[email protected]>

* chore: run `apt-get update` before running other commands (#41)

This will run `sudo apt-get update` before running any other commands on the Gitpod prebuild image (`.gitpod.dockerfile`).

* Readme: fix gitter link, add Nim logo, formatting fixes (#42)

* Update README.md

Use more extended markdown syntax instead of HTML,
Add Matrix links
Superseed PR #40

* Actually fix gitter! Move img src links to footer.

Also changed the TheAlgorithms logo image link.

* Restore TheAlgorithms logo link

* More link fixes

---------

Co-authored-by: dlesnoff <[email protected]>

* fix: nimpretty for `arc_length.nim` (#43)

* chore: `gc`->`mm`, move back to `config.nims` (#47)

* chore: revert #41 (#45)

* Update codeowners

* Fix: CONTRIBUTING.md links (#49)

Uppercase `directory.md` to `DIRECTORY.md`

* feat: add Levenshtein distance (#46)

* feat: add Levenshtein distance

* Apply suggestions from code review

Co-authored-by: Zoom <[email protected]>

* refactor: use toKey in initSubsolutions

* style: use .Natural

* style: remove blank line

* refactor: add Key type

* style: use descriptive variable names

* style: use a, b instead of strA, strB

* docs: add note about complexity

* refator: add name to TestCase

* docs: remove sentence about dp

* docs: update documentation of `levenshteinDistance`

Co-authored-by: Zoom <[email protected]>

* docs: rename initLevenshteinDistanceMatrix and add its documentation

* refactor: add fillLevenshteinDistanceMatrix

* tests: reorganise tests, remove TestCase type

* docs: add edit distance to module-level documentation

* docs: mention Wagner–Fischer algorithm

* docs: add comment in fillLevenshteinDistanceMatrix

* style: reformat fillLevenshteinDistanceMatrix

* docs: update DIRECTORY.md

---------

Co-authored-by: Zoom <[email protected]>

* docs: add missing `#` (#50)

* docs: add missing `#`

* docs: update DIRECTORY.md

* docs: remove ` of a Circle`

* docs: update DIRECTORY.md

---------

Co-authored-by: dlesnoff <[email protected]>

* Fix: Remove unuseful date command

* chore: Update checkout version to v4 in build_directory_md

* Add error status checking + separate compilation and execution of directory.nim

---------

Co-authored-by: Dimitri LESNOFF <[email protected]>
Co-authored-by: David Leal <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants