Skip to content

Readding the constants TOP, BOTTOM, LEFT_SIDE and RIGHT_SIDE #331

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

Closed
kolibril13 opened this issue Aug 22, 2020 · 2 comments
Closed

Readding the constants TOP, BOTTOM, LEFT_SIDE and RIGHT_SIDE #331

kolibril13 opened this issue Aug 22, 2020 · 2 comments
Labels
enhancement Additions and improvements in general

Comments

@kolibril13
Copy link
Member

In constans.py,
there once where the constants:

TOP = FRAME_Y_RADIUS * UP
BOTTOM = FRAME_Y_RADIUS * DOWN
LEFT_SIDE = FRAME_X_RADIUS * LEFT
RIGHT_SIDE = FRAME_X_RADIUS * RIGHT

I find them very useful, so I think we should include them back there.

When I am in a scene at the moment, my workaround is

        TOP = Camera().get_frame_height() / 2 * UP
        BOTTOM = Camera().get_frame_height() / 2 * DOWN
        LEFT_SIDE = Camera().get_frame_width() / 2 * LEFT
        RIGHT_SIDE = Camera().get_frame_width() / 2 * RIGHT
@kolibril13 kolibril13 added the enhancement Additions and improvements in general label Aug 22, 2020
@kolibril13
Copy link
Member Author

I just saw the conversation in #283 ,
and I also like the idea of

frame.top instead of the clunkier and less-readable camera_config["top"]

But as there is LEFT, UP and RIGHT, as a user I would expect that there would be as well e.g. TOP and BOTTOM.

@leotrs
Copy link
Contributor

leotrs commented Aug 22, 2020

Yes this is a tricky one. They had to be removed from constants.py because they aren't constants. But we don't want to get in the habit of populating the global manim namespace even more than it is now, especially not from the config.py file.

So my workaround is that all of those go into a new frame, but I don't think we have reached consensus there.

I guess we can put TOP and the others back in the manim namespace for the time being, for backwards compatibility.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Additions and improvements in general
Projects
None yet
Development

No branches or pull requests

2 participants