Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new transformation matrix Buw #115

Merged
merged 16 commits into from
Dec 5, 2023
Merged

Conversation

aaiaueil
Copy link
Contributor

@aaiaueil aaiaueil commented Dec 5, 2023

  • Linked issue related to your pull request using Transformation matrix are not all implemented #116
  • Used the tag [WIP] or [RTR] for on-going changes, or for ready to review
  • Did you write new tests to cover your changes?
  • Did you write a proper documentation (docstrings and ReadMe)
  • Please black your code black . -l120

This change is Reviewable

@aaiaueil aaiaueil changed the title Add new transformation matrix [RTR] Add new transformation matrix Dec 5, 2023
@aaiaueil aaiaueil changed the title [RTR] Add new transformation matrix [WIP] Add new transformation matrix Dec 5, 2023
@aaiaueil aaiaueil changed the title [WIP] Add new transformation matrix [RTR] Add new transformation matrix Dec 5, 2023
Copy link
Owner

@Ipuch Ipuch left a comment

Choose a reason for hiding this comment

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

Reviewed 1 of 2 files at r1, all commit messages.
Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @aaiaueil)


tests/test_transformation_matrix.py line 141 at r1 (raw file):

    TestUtils.assert_equal(bbox.compute_transformation_matrix(matrix_type=TransformationMatrixType.Bwu), res_Bwu)
    TestUtils.assert_equal(bbox.compute_transformation_matrix(matrix_type="Bwu"), res_Bwu)

Can you add a last test here ?

@Ipuch
Copy link
Owner

Ipuch commented Dec 5, 2023

  • The test doesn't pass, you have to run the evaluation with more numpy digits to ensure the test will pass.
  • Try to evaluate the transformation matrix with this command before numpy.set_printoptions(precision=20)

@Ipuch Ipuch changed the title [RTR] Add new transformation matrix [WIP] Add new transformation matrix Dec 5, 2023
Copy link
Contributor Author

@aaiaueil aaiaueil left a comment

Choose a reason for hiding this comment

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

Done

Reviewable status: 1 of 2 files reviewed, 1 unresolved discussion (waiting on @Ipuch)


tests/test_transformation_matrix.py line 141 at r1 (raw file):

Previously, Ipuch (Pierre Puchaud) wrote…

Can you add a last test here ?

Done.

Copy link

codeclimate bot commented Dec 5, 2023

Code Climate has analyzed commit c850036 and detected 0 issues on this pull request.

View more on Code Climate.

Copy link
Owner

@Ipuch Ipuch left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 2 files at r3, all commit messages.
Reviewable status: :shipit: complete! all files reviewed, all discussions resolved (waiting on @aaiaueil)

@Ipuch Ipuch merged commit b0e4c4c into Ipuch:main Dec 5, 2023
6 checks passed
@Ipuch Ipuch changed the title [WIP] Add new transformation matrix Add new transformation matrix Buw Dec 5, 2023
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