Skip to content

Conversation

@Tejasrahane
Copy link
Contributor

  • Improved type hints (int -> float)
  • Added edge case tests (angle=0, 360, 180)
  • Added float value tests
  • Added error handling for negative values
  • Enhanced docstring with formula and Wikipedia link
  • Added proper error messages

Contributes to #9943

Describe your change:

  • Add an algorithm?
  • Fix a bug or typo in an existing algorithm?
  • Add or change doctests? -- Note: Please avoid changing both code and tests in a single pull request.
  • 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".

Tejasrahane and others added 2 commits October 23, 2025 10:19
- Improved type hints (int -> float)
- Added edge case tests (angle=0, 360, 180)
- Added float value tests
- Added error handling for negative values
- Enhanced docstring with formula and Wikipedia link
- Added proper error messages

Contributes to TheAlgorithms#9943
@algorithms-keeper algorithms-keeper bot added the tests are failing Do not merge until tests pass label Oct 23, 2025
Copy link
Contributor Author

@Tejasrahane Tejasrahane left a comment

Choose a reason for hiding this comment

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

Great improvements to the arc_length.py file! Here's my review:

Strengths:

  1. Comprehensive Doctests: Excellent addition of edge case tests (angle=0, 360, 180) and error handling tests for negative values.

  2. Improved Type Hints: Changing from int to float in the function signature is correct since arc length calculations typically work with floating-point numbers.

  3. Better Documentation: The enhanced docstring with formula, Wikipedia link, and clear parameter descriptions makes the function much more understandable.

  4. Proper Error Handling: Adding ValueError for negative inputs with descriptive messages is a solid improvement.

Suggestions for Improvement:

  1. Doctest Import Location: Consider moving import doctest to the top of the file with other imports rather than at the bottom, following PEP 8 conventions.

  2. Float Value Tests: Some of the doctest examples use float inputs (e.g., arc_length(45.5, 10.5)), which is great for testing, but the expected output values should be verified for accuracy.

  3. Consider Testing Larger Angles: While you test 0, 180, and 360, it might be worth noting in the docstring whether angles > 360 are expected or should also raise an error.

Overall, this is a quality PR that significantly improves the code quality and testing coverage. Nice work!

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

Labels

tests are failing Do not merge until tests pass

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant