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

use upstream python-utils instead of vendoring #1558

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gagath
Copy link
Contributor

@gagath gagath commented Feb 25, 2021

Avoid vendoring by using upstream python-utils now that the project is correctly using PyPI.

@Harvie
Copy link
Collaborator

Harvie commented Feb 25, 2021

Didn't even knew there is such thing... Do you know what is this used for in bCNC? Perhaps we can remove it altogether...

@Harvie
Copy link
Collaborator

Harvie commented Feb 25, 2021

I've done some greping and i don't see it being imported anywhere, but maybe i am wrong.

@gagath
Copy link
Contributor Author

gagath commented Feb 25, 2021

It seems to be used by stl:

$ git grep python_utils
bCNC/lib/stl/base.py:from python_utils import logger

And stl seems to be only used by the slicemesh plugin:

git grep "import stl"
bCNC/lib/stl/main.py:from . import stl
bCNC/lib/stl/mesh.py:from . import stl
bCNC/plugins/slicemesh.py:import stl #FIXME: write smaller STL parser
bCNC/plugins/slicemesh.py:              #import stl

While a smaller STL parser is not introduced, this can reduce the amount of vendored code in the project.

@Harvie
Copy link
Collaborator

Harvie commented Feb 25, 2021

Ah, it seems to be used for some logging by that STL code i've nominated for cleanup in #1557 discussion.
Since the day one this was meant to be cleaned up and removed completely.

@Harvie
Copy link
Collaborator

Harvie commented Feb 25, 2021

If we add this to the setup.py, it will get installed on computers of all our users and never get uinstalled once we remove that code.

@gagath
Copy link
Contributor Author

gagath commented Feb 25, 2021

If we add this to the setup.py, it will get installed on computers of all our users and never get uinstalled once we remove that code.

It depends on how users install bCNC. If they install it using a Linux distribution or with proper pip commands, then they may be able to remove the dependencies aswell. Moreover, this is already the case for numpy, Pillow and all of the other depedencies that are already installed via setup.py.

@Harvie
Copy link
Collaborator

Harvie commented Sep 25, 2023

Anyway, in the meantime we can probably merge this, if you resolve the conflicts.

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

Successfully merging this pull request may close these issues.

2 participants