Skip to content

Add unit tests for fit_ellipse_to_points to address issue #36#39

Closed
onemriganka wants to merge 1 commit intoneuroinformatics-unit:mainfrom
onemriganka:add-fit-ellipse-tests
Closed

Add unit tests for fit_ellipse_to_points to address issue #36#39
onemriganka wants to merge 1 commit intoneuroinformatics-unit:mainfrom
onemriganka:add-fit-ellipse-tests

Conversation

@onemriganka
Copy link
Copy Markdown

fixes / #36

Description

What is this PR

  • Bug fix
  • Addition of a new feature
  • Other

Why is this PR needed?

This PR addresses issue [#36](#36) by adding dedicated unit tests for the fit_ellipse_to_points function. While the function is indirectly tested through integration tests, a specific unit test ensures that the ellipse center is correctly computed from a synthetic, well-defined set of points.

What does this PR do?

  • Introduces a new test file (tests/test_fit_ellipse.py) that:
    • Contains a helper function generate_ellipse_points to create a synthetic ellipse dataset with known parameters.
    • Implements a test (test_fit_ellipse_center) to verify that the computed ellipse center from fit_ellipse_to_points is within an acceptable tolerance of the expected center.
  • Updates the README with a "Running Tests" section, which provides clear instructions on installing dependencies and running tests using pytest.

References

How has this PR been tested?

The new unit test has been executed locally using pytest. The test suite passes without errors, ensuring that the new test does not affect any existing functionality.

Is this a breaking change?

No, this PR does not break any existing functionality.

Does this PR require an update to the documentation?

Yes. The README has been updated to include instructions for running the tests, making it easier for contributors to verify their changes.

Checklist:

  • The code has been tested locally
  • Tests have been added to cover all new functionality
  • The documentation has been updated to reflect any changes
  • The code has been formatted with [pre-commit](https://pre-commit.com/)

@lauraporta
Copy link
Copy Markdown
Member

Hi @onemriganka, thanks for your PR! I'm going to review it and give some feedback.

@lauraporta
Copy link
Copy Markdown
Member

@onemriganka, I see the linting check is not passing. Was the code formatted locally with pre-commit?
If not I can suggest you the steps.
Install the optional dependencies of derotation in your environment with pip install '.[dev]' if using zsh or pip install .[dev] for bash. Then run pre-commit install and pre-commit run --all-files. This should do some updates on the code and prompt you with changes you have to make yourself.

@onemriganka onemriganka closed this by deleting the head repository Jul 16, 2025
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