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

wrong description of rotate_left/right #158

Open
MathieuDerelle opened this issue May 31, 2019 · 4 comments
Open

wrong description of rotate_left/right #158

MathieuDerelle opened this issue May 31, 2019 · 4 comments

Comments

@MathieuDerelle
Copy link

MathieuDerelle commented May 31, 2019

the source code says

    # rotate the page 90 degrees counter clockwise
    def rotate_left

    # rotate the page 90 degrees clockwise
    def rotate_right

but from what I've tested, it's the opposite, rotate_left is rotating clockwise et rotate_right counter clockwise

@SlavaEremenko
Copy link

Confirming this issue!
Version 1.0.16

@boazsegev
Copy link
Owner

@MathieuDerelle , thank you for opening this issue.

@SlavaEremenko , thank you for confirming this issue.

Although I haven't tested this issue yet, I suspect that the biggest question I will have with this issue is "how to fix it?":

  1. I can fix the documentation, making things a bit awkward, since rotate_left implies counterclockwise motion.

  2. I can fix the API implementation, breaking current implementations that rely on the rotation direction.

I think this change / fix will require a version bump due to it being a breaking change.

I'll think about this some more and would be happy to read your input.

Kindly,
Bo.

@MathieuDerelle
Copy link
Author

MathieuDerelle commented Jul 3, 2019

I think rotate_left and rotate_right do not make much sense.

If you imagine a circle, rotating left could mean

  • rotate the point at the bottom of the circle to the left => clockwise
  • rotate the point at the top of the circle to the left => counter clockwise

You could define those 2 methods : rotate_clockwise / rotate_counter_clockwise
and deprecate rotate_left / rotate_right

@SlavaEremenko
Copy link

SlavaEremenko commented Jul 3, 2019

I agree with both of you. Keep the old methods, deprecate them, put a note into the docs and create new methods: rotate_cw and rotate_ccw.

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

No branches or pull requests

3 participants