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

RF: Flatten the window folder to the file #955

Merged
merged 2 commits into from
Dec 20, 2024

Conversation

maharshi-gor
Copy link
Contributor

RF: Flatten the window folder to the file

PR Contents

  • Flatten the window folder to the file.
  • Added all the pygfx imports from the window file to the lib.py.
  • Moved jupyter-rfb as an optional dependency under the plot.

- Flatten the window folder to the file.
- Added all the pygfx imports from window file to lib.
- Created jupyter-rfb as optional dependency under plot.
@maharshi-gor maharshi-gor self-assigned this Dec 20, 2024
@maharshi-gor
Copy link
Contributor Author

Hey @skoudoro , I have not added the docstring intentionally as It will change a lot in the upcoming PRs.

I have also figured out some tests, It will create a change in a lot of files and a lot of places. I am breaking this big change in to small changes, that are easy to review.

PS: This also tackles the issue of jupyter-rfb not being the optional dependency.

- Optional package used to trigger tripwire.
- Added pygfx and glfw to dependencies in pyproject.toml.
Comment on lines +26 to +30
# Classes that needed to be written as types
Camera: TypeAlias = gfx.Camera
Controller: TypeAlias = gfx.Controller
Scene: TypeAlias = gfx.Scene
Viewport: TypeAlias = gfx.Viewport
Copy link
Contributor

Choose a reason for hiding this comment

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

What do you need the TypeAlias here ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It gives pre-commit error as these classes are used to either extend the class or used as type in a dataclass.

Copy link
Contributor

Choose a reason for hiding this comment

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

strange, ok, I look into it later, merging

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@skoudoro you can take a look at this. The pre-commit points to this resource when it gives the error. https://mypy.readthedocs.io/en/stable/common_issues.html#variables-vs-type-aliases

Copy link
Contributor

@skoudoro skoudoro left a comment

Choose a reason for hiding this comment

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

all ok, merging

@skoudoro skoudoro merged commit f595400 into fury-gl:v2 Dec 20, 2024
3 of 22 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants