Skip to content

Updated maths/number_of_digits.py#2221

Merged
cclauss merged 11 commits intoTheAlgorithms:masterfrom
vasugamdha:patch-1
Jul 26, 2020
Merged

Updated maths/number_of_digits.py#2221
cclauss merged 11 commits intoTheAlgorithms:masterfrom
vasugamdha:patch-1

Conversation

@vasugamdha
Copy link
Copy Markdown
Contributor

@vasugamdha vasugamdha commented Jul 21, 2020 •
edited by poyea
Loading

Added two more methods!

Fixes: #2220
I have added 2 methods for maths/number_of_digits.py:

  1. num_digits2(): Also an O(n) algorithm approach.
  2. num_digits_fast(): An O(1) algorithm for counting the number of digits
  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Documentation change?

Checklist:

  • I have read CONTRIBUTING.md.
  • This pull request is all my own work -- I have not plagiarized.
  • I know that pull requests will not be merged if they fail the automated tests.
  • 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.
  • All functions have doctests that pass the automated testing.
  • All new algorithms have a URL in its comments that points to Wikipedia or other similar explanation.
  • If this pull request resolves one or more open issues then the commit message contains Fixes: #{$ISSUE_NO}.

closes #2220

@vasugamdha
Updated number_of_digits.py
213e764 
Added two more methods!
@TravisBuddy
Copy link
Copy Markdown

TravisBuddy commented Jul 21, 2020

Hey @vasugamdha,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 847ae9e0-cb88-11ea-96be-d1d226963d5e

@vasugamdha vasugamdha mentioned this pull request Jul 21, 2020
Closed
@vasugamdha
Copy link
Copy Markdown
Contributor Author

vasugamdha commented Jul 22, 2020

@cclauss I don't know why my PR is failing TravisCI checks. Please suggest some changes that need to be implemented to pass the checks. @TravisBuddy

@cclauss
Copy link
Copy Markdown
Member

cclauss commented Jul 22, 2020

https://github.com/TheAlgorithms/Python/blob/master/Travis_CI_tests_are_failing.md

@TravisBuddy
Copy link
Copy Markdown

TravisBuddy commented Jul 22, 2020

Hey @vasugamdha,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 09501ee0-cbe0-11ea-acda-6fc69b852bf0

@vasugamdha
Copy link
Copy Markdown
Contributor Author

vasugamdha commented Jul 22, 2020

@cclauss I have fixed the #2220 issue and it's ready for review and merge.

@vasugamdha
Copy link
Copy Markdown
Contributor Author

vasugamdha commented Jul 24, 2020

hello @cclauss , can you please merge this PR, as it has already passed both the checks and fixes the #2220 issue? Thanks.

@cclauss
Copy link
Copy Markdown
Member

cclauss commented Jul 24, 2020

Unfortunately, I see no benchmark code.

@vasugamdha
Copy link
Copy Markdown
Contributor Author

vasugamdha commented Jul 25, 2020

I haven't written benchmark code, as the function , I defined (num_digits_fast()), clearly returns number of digits in O(1). While the one already written function returns the same in O(n).

@cclauss
Copy link
Copy Markdown
Member

cclauss commented Jul 25, 2020 •
edited
Loading

What proof do you have for those theoretical statements without benchmarking them with the same data?
Here examples of algorithms with benchmarks using timeit: https://github.com/TheAlgorithms/Python/search?q=timeit

@TravisBuddy
Copy link
Copy Markdown

TravisBuddy commented Jul 26, 2020

Hey @vasugamdha,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 2b156200-cf01-11ea-9502-ed86ad10d0d8

@TravisBuddy
Copy link
Copy Markdown

TravisBuddy commented Jul 26, 2020

Hey @vasugamdha,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: 59091e10-cf04-11ea-9502-ed86ad10d0d8

@TravisBuddy
Copy link
Copy Markdown

TravisBuddy commented Jul 26, 2020

Travis tests have failed

Hey @vasugamdha,
Please read the following log in order to understand the failure reason.
It'll be awesome if you fix what's wrong and commit the changes.

TravisBuddy Request Identifier: bcddb020-cf06-11ea-9502-ed86ad10d0d8

@vasugamdha
Copy link
Copy Markdown
Contributor Author

vasugamdha commented Jul 26, 2020

First comment update!!

Added two more methods!

Fixes: #2220
I have added 2 methods for maths/number_of_digits.py, which are comparatively faster then already present method:

  1. num_digits_fast()
  2. num_digits_faster()

Here is the output of the benchmark code.

image

vasugamdha
vasugamdha commented Jul 26, 2020
View reviewed changes
Copy link
Copy Markdown
Contributor Author

@vasugamdha vasugamdha left a comment

Choose a reason for hiding this comment

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

@cclauss Please review it now!

@TravisBuddy
Copy link
Copy Markdown

TravisBuddy commented Jul 26, 2020

Hey @vasugamdha,
Something went wrong with the build.

TravisCI finished with status errored, which means the build failed because of something unrelated to the tests, such as a problem with a dependency or the build process itself.

View build log

TravisBuddy Request Identifier: d95a44a0-cf26-11ea-9502-ed86ad10d0d8

cclauss
cclauss approved these changes Jul 26, 2020
View reviewed changes
Copy link
Copy Markdown
Member

@cclauss cclauss left a comment

Choose a reason for hiding this comment

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

Impressive!!!

@cclauss cclauss merged commit e296f7b into TheAlgorithms:master Jul 26, 2020
stokhos pushed a commit to stokhos/Python that referenced this pull request Jan 3, 2021
@vasugamdha @stokhos
Updated maths/number_of_digits.py (TheAlgorithms#2221)
05cfc25 
* Updated number_of_digits.py

Added two more methods!

* Update number_of_digits.py

* Update number_of_digits.py

* Added benchmarks!

* Update number_of_digits.py

* Update number_of_digits.py

* Update number_of_digits.py

* Update number_of_digits.py

* Update number_of_digits.py

* Update number_of_digits.py

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

Reviewers

@cclauss cclauss cclauss approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

maths/number_of_digits.py is naive!

3 participants

@vasugamdha @TravisBuddy @cclauss