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

Reonsider dependencies #266

Open
rafaqz opened this issue Feb 24, 2025 · 7 comments
Open

Reonsider dependencies #266

rafaqz opened this issue Feb 24, 2025 · 7 comments

Comments

@rafaqz
Copy link
Member

rafaqz commented Feb 24, 2025

  • CoordinateTransformations should be easy to put in an extension? (it pulls in StaticArrays)
  • GeometryBasics is pulling in Earcut_jll and StaticArrays - do we actually need it for anything?

I just noticed adding GO to another package added all of these, it would be nice not to make that consideration a factor.

@asinghvi17
Copy link
Member

We may not need staticarrays, true, but mesh / triangulation support probably needs GeometryBasics. Especially spherical triangulation.

I'm going to try and reduce StaticArrays load times as well which should help a bit.

I currently use the CoordinateTransformations interface for spherical coordinates but we could remove that in theory.

@rafaqz
Copy link
Member Author

rafaqz commented Feb 24, 2025

Can we insulate everything else from meshes using an extension? Most basic work doesn't need it

Having a dependency on Earcut_jll is unnecessary for most of the package

@asinghvi17
Copy link
Member

True...we can't really insulate meshing from needing an extension but we might be able to remove GeometryBasics at least. I would still want to keep StaticArrays, at least for now. For triangulation we can always return the DelaunayTriangulation.jl object or maybe some tuple of points and faces.

@asinghvi17
Copy link
Member

Out of curiosity what prompted this?

@rafaqz
Copy link
Member Author

rafaqz commented Feb 24, 2025

Seeing the dependencies balloon when I added it to ConScape.jl just for polygon distance calculations.

I would prefer not to have any enforced binary deps, and I'm sure it will make other question adding it in similar contexts. In comparison Rasters is pretty light

@asinghvi17
Copy link
Member

Yeah this will also speed up loading GeometryOps by at least a third, probably more like a half.

Not that the load time was that bad anyway (0.3s on my machine, compared to 2.7s for Makie) but still...

The only thing I want to mention here is that we probably shouldn't shy away from including dependencies if it makes life significantly easier (e.g. DelaunayTriangulation, StaticArrays)

@rafaqz
Copy link
Member Author

rafaqz commented Feb 25, 2025

I shy away!

Mostly those binary deps. But as well as load time dependencies also increase compile time of every environment GO is used in.

(I guess I'm hoping GO can be lower in the stack, with many packages depending on it rather than just user facing. That means being careful with deps)

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

2 participants