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

fix rotate/extract ordering for non-90 angles #1755

Merged
merged 3 commits into from Jul 26, 2019
Merged

Conversation

iovdin
Copy link
Contributor

@iovdin iovdin commented Jun 18, 2019

Fixes #1682.
Without refactoring as noted in the issue

@lovell
Copy link
Owner

lovell commented Jun 18, 2019

Hi, thanks for this. Are you able to add a unit test or two that would previously have failed. There is a linter error to deal with also.

@iovdin
Copy link
Contributor Author

iovdin commented Jun 19, 2019

will do

@coveralls
Copy link

Coverage Status

Coverage remained the same at 97.445% when pulling 570d1bb on iovdin:master into cfa4f7d on lovell:master.

@iovdin
Copy link
Contributor Author

iovdin commented Jun 25, 2019

@lovell Does it looks good enough?

@lovell
Copy link
Owner

lovell commented Jun 25, 2019

Sorry, I somehow missed the update notifications for this - will take a look tomorrow - thanks again for fixing it.

@lovell
Copy link
Owner

lovell commented Jun 26, 2019

Thanks for fixing the linter error and adding tests. Are you able to move the copy/pasted section of code that performs the rotation into a common Rotate function in the operations.cc file?

@lovell lovell changed the base branch from master to vision July 26, 2019 19:19
@lovell lovell merged commit 690bc43 into lovell:vision Jul 26, 2019
@lovell
Copy link
Owner

lovell commented Jul 26, 2019

Thanks for this PR and for bearing with me whilst I prepared the rest of the next release. This will be in v0.23.0.

@lovell lovell added this to the v0.23.0 milestone Jul 26, 2019
lovell added a commit that referenced this pull request Jul 28, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants