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

make meshio optional #1938

Closed
nschloe opened this issue Dec 12, 2021 · 14 comments · Fixed by #1939
Closed

make meshio optional #1938

nschloe opened this issue Dec 12, 2021 · 14 comments · Fixed by #1939
Labels
feature-request Please add this cool feature!

Comments

@nschloe
Copy link
Contributor

nschloe commented Dec 12, 2021

pyvista is lagging behind with meshio dependencies. This is fine, but poses problems in environment where I must use the latest version. I'd hence like to suggest to make meshio an optional dependency of pyvista.

@nschloe nschloe added the feature-request Please add this cool feature! label Dec 12, 2021
@adeak
Copy link
Member

adeak commented Dec 12, 2021

The <5 constraint was added in #621, February 2020. You released 5.0 this August. I think this is just a case of overly cautious version specifications; perhaps it would be better to just remove that constraint from setup.py altogether, and only introduce such version upper bounds when we have a good reason to suspect future breakage.

@pyvista/developers thoughts?

Alternatively, we could set up something like dependabot to tell us about upstream updates so we can bump versions.

@nschloe
Copy link
Contributor Author

nschloe commented Dec 12, 2021

There's soon be another upgrade, and a bigger one, too.

@MatthewFlamm
Copy link
Contributor

PyVista is "3D plotting and mesh analysis through a streamlined interface for the Visualization Toolkit (VTK)".

meshio seems like a useful addition, but not an essential dependency. There are many possible workflows that would not require meshio.

I'm +1 on making it optional.

I like the idea of automated dependabot if there is pinning. I think the pinning strategy should be consistent barring some known incompatibility. For example, numpy isn't pinned to <2.0.0. This seems like a second discussion to me however.

@banesullivan
Copy link
Member

I think making meshio optional is a smart move, but I also think letting users load just about any mesh format out of the box between PyVista's VTK readers and meshio's readers is incredibly useful. Plus meshio is pure Python so besides the versioning issues, it's an incredibly lightweight dependency.

I'm wondering if instead, we can come up with a "contract" between PyVista and meshio so that the general interface that PyVista uses stays consistent across versions. @nschloe, would that be a big ask? I may have some bandwidth to help implement such an interface/contract. That way we can just free the meshio dependency and not worry about this moving forward

@nschloe
Copy link
Contributor Author

nschloe commented Dec 12, 2021

I don't think I understand. What do you mean by "contract"?

@nschloe
Copy link
Contributor Author

nschloe commented Dec 12, 2021

@tkoyama010
Copy link
Member

"contract"

I understood this to be creating a data object to be exchanged between meshio and PyVista (@banesullivan Is this right? ). If that is the case, I agree with this idea. I was uncomfortable with the fact that PyVista calls and reads meshio internally. I think the natural flow is to use meshio to load the mesh and then use the data object to visualize it in PyVista.

@nschloe
Copy link
Contributor Author

nschloe commented Dec 13, 2021

I think this is a separate discussion.

@nschloe
Copy link
Contributor Author

nschloe commented Dec 15, 2021

@banesullivan Could you detail on what you mean by "contract"?

@banesullivan
Copy link
Member

banesullivan commented Dec 16, 2021

What do you mean by "contract"?

I understood this to be creating a data object to be exchanged between meshio and PyVista

More or less, yes -- this is what I was getting after.

It is my opinion that meshio and pyvista are in a symbiotic relationship: pyvista gains from meshio's IO support for formats we/VTK do not support and meshio benefits from pyvista providing a simple to use API for plotting and analysis (to do stuff with the data meshio reads).

I'm wondering if we can lock in that relationship and implement a test upstream in meshio that effectively says "hey, this is the way PyVista is using meshio, let's try not to break this part of the API". That way, pyvista can have no limit on the meshio dependency and if the API changes/that test has to change it will be more clear on how to address downstream in PyVista.

and I'm wondering if we can simplify this method as much as possible:

def from_meshio(mesh):

@banesullivan
Copy link
Member

In the meantime, I am +1 for merging #1939 and leaving this issue open to figure out how to improve the interoperability between meshio and pyvista

@adeak
Copy link
Member

adeak commented Dec 16, 2021

I think pyvista is clearly downstream from meshio. Meshio could do whatever it wants with its meshes. It would be weird for meshio to specifically assimilate like that.

I believe the best we could ask for is a well-defined public API that's hopefully not too unstable (but I'd note that we break things all the time too). We could then test the public API ourselves and find breakage in CI, before any release. Still no pinning needed.

@tkoyama010
Copy link
Member

I'm sorry that my argument may have come across as burdensome to meshio. My image is matplotlib for numpy's array object, numpy doesn't need to care about matplotlib (as far as I know), and that's my stance on meshio as well. In any case, it's not something that should be discussed here, so I'll create a new Discussion.

@tkoyama010
Copy link
Member

Please discuss in #1946.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request Please add this cool feature!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants