-
Notifications
You must be signed in to change notification settings - Fork 28
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
Move all pyvista specific code into pyvista #21
Comments
It's all because of the license on MeshFix. See #7 and a similar details in pyvista/pyvista#240 |
Indeed, it is incredibly backwards. I would recommend emailing the creators of the original MeshFix software to let them know your thoughts on their license choice and how it's hindering adoption and usage of the software. |
I'm actually of the opinion of keeping each module as independent as possible. While The
This was actually a tough choice to make. VTK isn't a light dependency, and they were a year late for a Python 3.8 release (and will probably be for Python 3.9). I could remove |
Also, PyVista is already a soft dependency here... it's listed in the pymeshfix/pymeshfix/meshfix.py Lines 10 to 14 in b673532
|
I suppose we could make different extras dependency options in the setup.py so that if a downstream project wants to depend on pymeshfix without pyvista they could do that... but it would go against #16 (comment) because you cannot have Perhaps we have a |
Perhaps, but that might still violate |
To clarify, that |
Thank you for the discussion and clarification on the issues with the MeshFix license. My first thought was indeed to make pyvista an extras_require option. But I understand that for normal python users, it is not common to install a package with ‘pip install pymeshfix[all]’. Another option could be to split the pymeshfix package into 2 subpackages: pymeshfix-core and pymeshfix-vista. The pymeshfix.core would only contain the wrappers of MeshFix and any pyvista related functionality would be in pymeshfix.vista. This subpackage structure creates 2 wheels pymeshfix-core and pymeshfix-vista and can have their own required dependencies. These 2 packages are separated on pypi, but once installed they act as submodules from the same pymeshfix package. |
If pyvista is specified in the setup.py file under install_requires (which is the case), it is not a soft dependency. I agree that if pyvista is not installed, the code treats it as a soft dependency, but as a package, pymeshfix has a hard requirement on pyvista. If I want to create a library that depends on pymeshfix, there is no way the users of my library can install my library using |
Fair point... it'll have to be up to @akaszynski on whether we split this up into a |
On further thought, I'm not terribly opposed to just making PyVista an optional depend in |
It wouldn't be too hard to split up |
Yeah, I sure do not have the time to help with splitting it up. I'd be in favor of just dropping the pyvista depend for the sake of time/effort and having a clear notice in the installation instructions that people probably also want pyvista. e.g.:
|
Maybe that's best. Just a simple |
If that would be helpful, I can create a PR with the changes in the setup file and changes in the readme. |
That's ok. There's more than just the setup changes as I need to run unit testing without installing |
It is currently impossible to use the core of the pymeshfix without installing pyvista. It is true that using
pip install pymeshfix --nodependencies
will not install pyvista (see #16 ), but this is not possible when pyvista is part of a requirements.txt file for example. Also, we don't want no dependencies at all, we just don't want any unnecessary dependencies for the core of the library (like pyvista, depending on imageio, appdirs, scooby, meshio and vtk).To me it seems backwards that pymeshfix has a pyvista interface at all, and this code is not just included into pyvista. pymeshfix could be specified as a pyvista requirement and all plotting and vtk interfaces could be implented at the pyvista side.
This would imo be much more logical because you will not have to deal with any optional requirements anymore. If you want to use the core of pymeshfix, you just install pymeshfix. If you want to use a pyvista interface and fix meshes and plot them, use pyvista, which includes pymeshfix as a utility.
Is there any reason why pymeshfix is not a dependency of pyvista?
I really believe the functionality in pymeshfix is very useful. However, the unnecessary dependency of pyvista will push away people that are only looking for a way to fix meshes or perform mesh validity checks.
I would be willing to contribute and create a pull request for both pymeshfix and pyvista with the necessary changes.
The text was updated successfully, but these errors were encountered: