Skip to content

add atbash, crout, hosoya triangle algorithms#470

Merged
keon merged 2 commits into
keon:masterfrom
Notheryne:add-atbash-crout-hosoyatriagle
Jan 24, 2019
Merged

add atbash, crout, hosoya triangle algorithms#470
keon merged 2 commits into
keon:masterfrom
Notheryne:add-atbash-crout-hosoyatriagle

Conversation

@Notheryne
Copy link
Copy Markdown
Contributor

@Notheryne Notheryne commented Jan 17, 2019

(Put an X inside the [ ] to denote check mark [X].)

  • If creating a new file :

    • added links to it in the README files ?
    • included tests with it ?
    • added description (overview of algorithm, time and space compleixty, and possible edge case) in docstrings ?
  • if done some changes :

    • wrote short description in the PR explaining what the changes do ?
    • Fixes #[issue number] if related to any issue
  • other

Copy link
Copy Markdown
Owner

@keon keon left a comment

Choose a reason for hiding this comment

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

thanks for your contribution!
I added some minor comments on naming convention.

I'd be nice to have some unit tests as well.

Comment thread algorithms/dp/hosoya_triangle.py Outdated



def Hosoya(n, m):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

hosoya

Comment thread algorithms/dp/hosoya_triangle.py Outdated
else:
return 0

def printHosoya(n):
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

print_hosoya

@Notheryne
Copy link
Copy Markdown
Contributor Author

Ok, I'll work on the tests throughout the weekend, I should be able to include them by sunday.

@Notheryne
Copy link
Copy Markdown
Contributor Author

I've added the unittests, updated non-english readmes, made some descriptions better and changed crout to always return floats.
I had to make a new function in hosoya for testing, because it is supposed to only print. Hope it's good.

@keon keon merged commit d31ec0a into keon:master Jan 24, 2019
@keon
Copy link
Copy Markdown
Owner

keon commented Jan 24, 2019

@Notheryne Thanks for the contribution!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants