Feature_A_6#12
Conversation
added init py and binop module
FeatureA_6 core functions
FeatureA_6 Basic Arithmetic Functions
Add input validation for binary strings
Update binary.py by deepjit
Revert "Update binary.py by deepjit"
Revert "Add input validation for binary strings"
Added execption.py for input validation.
feature: add binary complement operations
calculator.py now has function drivers for evaluating binary operations.
Feature a 6 tests
Binary Operation Integration with Base Calculator Operation .py file
fixed duplicate divide.
fixed import path for exceptions module
added imports for multiply and divide
Removed project structure section from README.
fixed import path for exceptions module
fixed subtraction function
fixed ones complement function: issue: modular arithmetic kills the carry fix: let the carry naturally extend the bit length
There was a problem hiding this comment.
Pull request overview
Adds a binary-number feature set to the existing calculator codebase, including conversion utilities, binary arithmetic/complement operations, custom exceptions, and CI coverage for the new module.
Changes:
- Introduces
modules/binary.py(+ re-exports) andmodules/exceptions.pyfor binary operations and domain-specific errors. - Extends
Calculatorwith anevaluate()string router to invoke binary features and arithmetic via simple expression parsing. - Adds unit tests for the binary module and updates CI workflow to run tests and report coverage.
Reviewed changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
calculator.py |
Adds imports for binary features and implements Calculator.evaluate() expression routing/parsing. |
modules/binary.py |
Implements binary↔decimal conversion, binary arithmetic, and 1’s/2’s complement helpers. |
modules/exceptions.py |
Adds custom exceptions for invalid binary input and negative subtraction results. |
modules/__init__.py |
Re-exports binary functions at the package level. |
tests/testbinary.py |
Adds unit tests covering conversions, arithmetic, complements, and invalid input. |
tests/__init__.py |
Exposes tests via star import (package init). |
test_calculator.py |
Renames duplicate divide tests to distinct names. |
README.md |
Documents binary features, input format, and how to run tests. |
.github/workflows/tests.yml |
Adds GitHub Actions workflow to run tests and produce a coverage report. |
.gitignore |
Adds common Python/IDE/OS ignores. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Refactor Calculator class by removing unused imports and methods, and adding a mode attribute for future extensions.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import re | ||
| import sys | ||
|
|
There was a problem hiding this comment.
re and sys are imported but never used in this module. Unused imports can trigger linting failures and make dependencies unclear; please remove them (or use them if intentionally needed).
| import re | |
| import sys |
|
|
||
| return Calculator.evaluate(self.mode) |
There was a problem hiding this comment.
When self.mode != 2, this method calls Calculator.evaluate(self.mode), which ignores the provided expression and (given the current base implementation) just prints and returns None. This makes Binary.evaluate(...) silently return None outside binary mode; consider either delegating to a properly-typed base evaluate(self, expression) implementation or raising a clear error for unsupported modes.
| return Calculator.evaluate(self.mode) | |
| raise ValueError(f"Unsupported mode for Binary.evaluate: {self.mode}") |
|
|
||
| def test_missing_prefix(self): | ||
| with self.assertRaises(InvalidBinaryInputError): | ||
| binary_to_decimal("1021") |
There was a problem hiding this comment.
test_missing_prefix currently passes "1021", which would raise InvalidBinaryInputError due to the invalid digit 2 even if the missing prefix were accepted. If the intent is to require the B' prefix, add a case like "1010" (valid bits but missing prefix) or adjust the test name/expectation to match the actual validation behavior.
| binary_to_decimal("1021") | |
| binary_to_decimal("1010") |
| if [ -f requirements.txt ]; then pip install -r requirements.txt; fi # Installs any dependencies you have | ||
| - name: Test with unittest | ||
| run: python -m unittest discover # Runs the unittest discover command | ||
| run: PYTHONPATH=. python -m unittest discover # Runs the unittest discover command |
There was a problem hiding this comment.
python -m unittest discover uses the default pattern test*.py, so tests/binary_tests.py will not be executed in this workflow. Either rename the test file to match test*.py (e.g., tests/test_binary.py) or update the discover command to use a pattern that includes *_tests.py and/or the tests/ start directory.
| run: PYTHONPATH=. python -m unittest discover # Runs the unittest discover command | |
| run: PYTHONPATH=. python -m unittest discover -s tests -p "*test*.py" # Runs unittest discovery against the tests directory and includes files such as test_*.py and *_tests.py |
Integration of Binary Operations to the Base Calculator.