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

Rename Point3f0 and similar to Point3f, etc. #61

Closed
knuesel opened this issue Jun 9, 2020 · 9 comments
Closed

Rename Point3f0 and similar to Point3f, etc. #61

knuesel opened this issue Jun 9, 2020 · 9 comments

Comments

@knuesel
Copy link
Contributor

knuesel commented Jun 9, 2020

I'd like to make a case against the suffix 0 in types such as Point3f0:

Is there a reason to prefer Point3f0?

@pauljurczak
Copy link
Contributor

As one of the beginners in questions, I fully agree. In the C++ world, you will find almost exclusively Point3f, Vector3f, etc. notation. For different element types, you will see: Point3d, Point3i, etc. The first thing I did after seeing Point3f0 was:

julia> Point3f
ERROR: UndefVarError: Point3f not defined

@SimonDanisch
Copy link
Member

I wouldn't mind in general - but I would mind the work to refactor this and properly get rid of it in all dependencies :D

@pauljurczak
Copy link
Contributor

How about an official Point3f alias?

@knuesel
Copy link
Contributor Author

knuesel commented Jun 9, 2020

@SimonDanisch I can imagine it would be a lot of work, especially with all the dependent projects. Of course if the change is desired, it's better to do it as soon as possible. I would be happy to help with the refactoring (I could work on PRs in the coming weeks if there's interest).

@pauljurczak I'd rather not have both, it introduces another kind of confusion. But I guess we would have an alias during a deprecation period.

@jkrumbiegel
Copy link

I would mind the work to refactor this and properly get rid of it in all dependencies :D

I can't imagine this would be so much work, after all, it's a prime candidate for a simple search and replace, no? Of course tagging the versions would be a bit of work. I like the idea of a rename, especially because we would match the conventional naming, and we should do things like this earlier rather than later.

@SimonDanisch
Copy link
Member

Yeah, its not a crazy amount! I mainly meant to say, that I won't do it in the little time i have, but if someone is willing to, they're free to open a PR to the dependent packages :)

@asinghvi17
Copy link
Contributor

I think a depwarn release of GeometryBasics + a replace in dependent packages should be sufficient. Here's the list of all (direct and indirect) dependent packages:

AbstractPlotting
Arena
AugmentedGaussianProcesses
CairoMakie
CatmullClark
ComplexPhasePortrait
ConstrainedDynamics
ConstrainedDynamicsVis
DrakeVisualizer
GLMakie
GeoMakie
ImplicitPlots
InteractiveChaos
Makie
MakieGallery
MakieLayout
MakieThemes
MechanismGeometries
MeshCatMechanisms
MicroscopyLabels
PredictMDExtra
PredictMDFull
RigidBodyTreeInspector
StatsMakie
TrajectoryOptimization
WGLMakie

(from JuliaHub)

@knuesel
Copy link
Contributor Author

knuesel commented Jun 12, 2020

@jkrumbiegel it would take a bit more care at least in GeometryBasics since the type names are macro-generated, but that might make it even easier... So yeah I guess most of the work would be in filing PRs for the different packages. I think I could do it some time this month.

@knuesel
Copy link
Contributor Author

knuesel commented Jul 20, 2021

(Oops I was a bit fast, it's not merged yet.)

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

5 participants