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

Elliptical Cylinder Geometry #1682

Open
tylerflex opened this issue May 6, 2024 · 8 comments
Open

Elliptical Cylinder Geometry #1682

tylerflex opened this issue May 6, 2024 · 8 comments
Labels
feature enables new functionality

Comments

@tylerflex
Copy link
Collaborator

No description provided.

@tylerflex tylerflex added the feature enables new functionality label May 6, 2024
@momchil-flex
Copy link
Collaborator

I think maybe instead of a separate Geometry class this could be a Cylinder classmethod that creates a transformed cylinder?

@tylerflex
Copy link
Collaborator Author

I had the same idea. @e-g-melo mentioned it would be better for gui integration if we made it its own class. At the same time, we don't want to clutter the geometry primitives so I'm not sure

@momchil-flex
Copy link
Collaborator

The classmethod approach would also be easier for the backend, as in, no changes needed, vs. needing to implement an Ellipse there too otherwise. But at the end of the day maybe we should do whatever makes the GUI experience best.

@tylerflex
Copy link
Collaborator Author

What would be nice is a way for GuI to offer ways to enter fields using class method constructors. But then the original data is lost when we load the model back

@e-g-melo
Copy link
Collaborator

e-g-melo commented May 7, 2024

A native Ellipse would be ideal for GUI, but that is not super urgent. Two academic users have asked for that, so if you decide to implement it, we could have this as a lower-priority task.

@tylerflex
Copy link
Collaborator Author

maybe we can offer a classmethod for now? should be a super simple task and potentially good one for new people to implement to get familiar with the code.

@e-g-melo
Copy link
Collaborator

e-g-melo commented May 7, 2024

I think so. It will certainly add. It would be nice if we could make it so that users easily understand they are getting a transformed object.

@tylerflex
Copy link
Collaborator Author

It will be in the type annotation shown in the docs

def elliptical(...) -> Transformed:
    ...

however, it might be a circular import if Transformed ever imports Cylinder, so perhaps we can't technically do this. It might rather need to be a class method of Transformed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature enables new functionality
Projects
None yet
Development

No branches or pull requests

3 participants