ufosc / manim-data-structures

A Manim plugin that contains common data structures to create Manimations.
https://manim-data-structures.readthedocs.io
MIT License
2 stars 26 forks source link

Remove enum usage #8

Open Nikhil-42 opened 1 year ago

Nikhil-42 commented 1 year ago

Description

The use of enums here to implement getters, setters, and mutators is not necessary and decreases readability. For example, instead of fetch_mob(MArrayElementComp) it is much better simply to have 4 methods get_body(), get_value(), get_index(), and get_label(). The 4 methods are self-documenting and the resulting code is actually shorter.

Although I like the use of enums for specifying the directions (MArrayDirection), Manim has adopted the use of standard numpy arrays (UP, DOWN, LEFT, RIGHT) to denote directions. The use of an additional enum is unnecessary and doesn't conform with Manim practices.

TODO

Zain3 commented 5 months ago

Im taking Intro to SWE, and we have to make a contribution to an Open source Project. So, ima do this

adalysg commented 1 month ago

Hello! Can you possibly specify a bit more how you'd like for the usage of MArrayDirection to be removed?

Nikhil-42 commented 1 month ago

Hello! Can you possibly specify a bit more how you'd like for the usage of MArrayDirection to be removed?

Instead of creating a new enum MArrayDirection for UP, DOWN, LEFT, RIGHT we should just be checking for equality with from manim import UP, DOWN, LEFT, RIGHT

adalysg commented 1 month ago

Thank you for your response! So this is my first contribution to an OS project, so I hope you understand if I still have a few questions. If I'm understanding correctly, anytime I see something along the lines of MArrayDirection.RIGHT (or that otherwise tries to access a member the enum), I should simply rewrite/replace with the RIGHT constant, as imported from manim?

Nikhil-42 commented 1 month ago

Thank you for your response! So this is my first contribution to an OS project, so I hope you understand if I still have a few questions. If I'm understanding correctly, anytime I see something along the lines of MArrayDirection.RIGHT (or that otherwise tries to access a member the enum), I should simply rewrite/replace with the RIGHT constant, as imported from manim?

Yes that's the idea, if you want more information please connect with UF's Open Source club on discord.