Octal to Binary Convert

[BamaCharanChhandogi]

Describe your change:

• 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 include at least one URL that points to Wikipedia or another similar explanation.
• 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".

[algorithms-keeper]

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

• Contributing guidelines
• Project Euler solution guidelines

Python:

• Formatted string literals (f-strings)
• Type hints
• doctest
• unittest
• pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

---

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

• @algorithms-keeper review to trigger the checks for only added pull request files
• @algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

[algorithms-keeper]

Please provide return type hint for the function: octal_to_binary. If the function does not return a value, please provide the type hint as: def function() -> None:

Please provide type hint for the parameter: octal_number

[BamaCharanChhandogi]

check again

[rohan472000]

You have used int() and bin() functions which are built-in functions in Python used for converting between different number bases.

In this repo, all function should be implemented from scratch without using built-in functions.

[tianyizheng02]

As already stated, this implementation just uses built-in Python functions to do all the work of the conversion. Plus, we'd rather have an implementation that converts directly from octal to binary without resorting to converting to decimal first.

[algorithms-keeper]

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

• Contributing guidelines
• Project Euler solution guidelines

Python:

• Formatted string literals (f-strings)
• Type hints
• doctest
• unittest
• pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

---

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

• @algorithms-keeper review to trigger the checks for only added pull request files
• @algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

[algorithms-keeper]

Please provide return type hint for the function: octal_to_binary. If the function does not return a value, please provide the type hint as: def function() -> None:

As there is no test file in this pull request nor any test function or class in the file conversions/octal_to_binary.py, please provide doctest for the function octal_to_binary

Please provide type hint for the parameter: octal_number

[BamaCharanChhandogi]

Check!

[algorithms-keeper]

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

• Contributing guidelines
• Project Euler solution guidelines

Python:

• Formatted string literals (f-strings)
• Type hints
• doctest
• unittest
• pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

---

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

• @algorithms-keeper review to trigger the checks for only added pull request files
• @algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

[algorithms-keeper]

Click here to look at the relevant links ⬇️

🔗 Relevant Links

Repository:

• Contributing guidelines
• Project Euler solution guidelines

Python:

• Formatted string literals (f-strings)
• Type hints
• doctest
• unittest
• pytest

Automated review generated by algorithms-keeper. If there's any problem regarding this review, please open an issue about it.

---

algorithms-keeper commands and options

algorithms-keeper actions can be triggered by commenting on this PR:

• @algorithms-keeper review to trigger the checks for only added pull request files
• @algorithms-keeper review-all to trigger the checks for all the pull request files, including the modified files. As we cannot post review comments on lines not part of the diff, this command will post all the messages in one comment.

NOTE: Commands are in beta and so this feature is restricted only to a member or owner of the organization.

[tianyizheng02]

Minor tweaks in the header docstring, but other than that LGTM

[tianyizheng02]

Formatting and grammar fixes

Suggested change

	reference for better understand
	URL: https://en.wikipedia.org/wiki/Binary_number
	URL: https://en.wikipedia.org/wiki/Octal
	References for better understanding:
	https://en.wikipedia.org/wiki/Binary_number
	https://en.wikipedia.org/wiki/Octal

[tianyizheng02]

Suggested change

	binary_number = ""
	octal_digits = "01234567"
	"""
	ValueError: String to the function
	>>> oct_to_decimal("Av")
	Traceback (most recent call last):
	...
	ValueError: Non-octal value was passed to the function
	>>> oct_to_decimal("90")
	Traceback (most recent call last):
	...
	ValueError: Special Character was passed to the function
	>>> oct_to_decimal("#$")
	Traceback (most recent call last):
	...
	ValueError: Empty String was passed to the function
	>>> oct_to_decimal("")
	...
	ValueError: octal value was passed to the function
	>>> oct_to_decimal("17")
	001111
	>>> oct_to_decimal("7")
	111
	"""
	"""
	ValueError: String to the function
	>>> oct_to_decimal("Av")
	Traceback (most recent call last):
	...
	ValueError: Non-octal value was passed to the function
	>>> oct_to_decimal("90")
	Traceback (most recent call last):
	...
	ValueError: Special Character was passed to the function
	>>> oct_to_decimal("#$")
	Traceback (most recent call last):
	...
	ValueError: Empty String was passed to the function
	>>> oct_to_decimal("")
	...
	ValueError: octal value was passed to the function
	>>> oct_to_decimal("17")
	001111
	>>> oct_to_decimal("7")
	111
	"""
	binary_number = ""
	octal_digits = "01234567"

Just noticed that the docstring is in the wrong place—I don't think these tests even pass because the error messages are wrong

[tianyizheng02]

Please fix your broken doctests—they're breaking because they're wrongly formatted and because your function never raises the right ValueError in each case. Refer to the documentation for the doctest library for more info.

[tianyizheng02]

Suggested change

Please remove this empty line

[BamaCharanChhandogi]

I have completed all documents and test cases.
but what is "process completed with exit code 1"?

[tianyizheng02]

I have completed all documents and test cases. but what is "process completed with exit code 1"?

It means the build experienced an error. See the build logs:

=================================== FAILURES ===================================
____________ [doctest] conversions.octal_to_binary.octal_to_binary _____________
012 
013     Convert an Octal number to Binary.
014 
015     ValueError: Non-octal value was passed to the function
016     >>> octal_to_binary("Av")
Differences (unified diff with -expected +actual):
    @@ -1,3 +1,8 @@
     Traceback (most recent call last):
    -    ...
    -ValueError: Non-octal value was passed to the function
    +  File "/opt/hostedtoolcache/Python/3.11.4/x64/lib/python3.11/doctest.py", line 1351, in __run
    +    exec(compile(example.source, filename, "single",
    +  File "<doctest conversions.octal_to_binary.octal_to_binary[0]>", line 1, in <module>
    +    octal_to_binary("Av")
    +  File "/home/runner/work/Python/Python/conversions/octal_to_binary.py", line 46, in octal_to_binary
    +    raise ValueError("Special Character was passed to the function")
    +ValueError: Special Character was passed to the function

It was expecting to get ValueError: Non-octal value was passed to the function but it instead got ValueError: Special Character was passed to the function

[tianyizheng02]

Suggested change

	raise ValueError("Empty String was passed to the function")
	raise ValueError("Empty string was passed to the function")

[tianyizheng02]

Suggested change

	for digit in octal_number:
	if not digit.isdigit():
	raise ValueError("Special Character was passed to the function")
	if digit < "0" or digit > "7":
	raise ValueError("Non-octal value was passed to the function")

These checks are unnecessary because you're already checking each character in the for-loop

[tianyizheng02]

Suggested change

	raise ValueError("Octal value was passed to the function")
	raise ValueError("Non-octal value was passed to the function")

This error message is incorrect

[tianyizheng02]

Suggested change

	"""
	Convert an Octal number to Binary.
	ValueError: Non-octal value was passed to the function
	>>> octal_to_binary("Av")
	Traceback (most recent call last):
	...
	ValueError: Non-octal value was passed to the function
	ValueError: Special Character was passed to the function
	>>> octal_to_binary("@#")
	Traceback (most recent call last):
	...
	ValueError: Special Character was passed to the function
	ValueError: Empty String was passed to the function
	>>> octal_to_binary("")
	Traceback (most recent call last):
	...
	ValueError: Empty String was passed to the function
	ValueError: Octal value was passed to the function
	>>> octal_to_binary("17")
	'001111'
	ValueError: Octal value was passed to the function
	>>> octal_to_binary("7")
	'111'
	"""
	"""
	Convert an Octal number to Binary.
	>>> octal_to_binary("17")
	'001111'
	>>> octal_to_binary("7")
	'111'
	>>> octal_to_binary("Av")
	Traceback (most recent call last):
	...
	ValueError: Non-octal value was passed to the function
	>>> octal_to_binary("@#")
	Traceback (most recent call last):
	...
	ValueError: Non-octal value was passed to the function
	>>> octal_to_binary("")
	Traceback (most recent call last):
	...
	ValueError: Empty string was passed to the function
	"""

Place the most import doctests at the top (the ones that demonstrate correct output) and edge cases should come after those. Also, there's no need to describe the error when the doctests already shows the error messages.

[BamaCharanChhandogi]

I understand. I am saying that If I remove the Special character test case!
I don't think there need for this type of algo.

[tianyizheng02]

My suggestions do not remove the special character test case. That doctest is still there. My point is that you had already handled special characters in your for-loop, so there's no need to handle it ahead of time before the for-loop.

[BamaCharanChhandogi]

Hello, @tianyizheng02 Finally could you check?

[tianyizheng02]

Just some minor formatting issues, but other than that LGTM