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

[FEATURE]: Implement support for concurrent installation with vaab/colour Pypi package. #1221

Open
Voultapher opened this issue Nov 6, 2023 · 7 comments

Comments

@Voultapher
Copy link

Description

Recently I ran into #958 while wanting to use manim and colour-science in the same Python project. The workaround is not great. I'd appreciate it if you could build an additional package and release that together with the coluor-science package. Suggested feature:

pip install colour-science
>>> import colour

pip install colour-science-namespaced
>>> import colour_science
@KelSolaar KelSolaar changed the title Add release that doesn't collide with the colour package [FEATURE]: Implement support for concurrent installation with the colour Pypi package. Nov 7, 2023
@KelSolaar
Copy link
Member

I thought about this one and I think there is an easy way to solve it: So basically, our colour is an actual package whilst the Pypi colour package is a single module, it has also not received any updates in a while which probably makes it very stable. With that in mind and because packages do take precedence over module, I think that we should be able to import the colour Pypi module objects into our namespace. I will take a better look but I don't think there will be clashes. It is properly disgusting but it should work.

@KelSolaar
Copy link
Member

References vaab/colour#62

@Voultapher
Copy link
Author

Assuming it works as intended, I'm fine with the suggested fix. Certainly not clean or pretty, but it would be an improvement over the current status quo.

@KelSolaar KelSolaar changed the title [FEATURE]: Implement support for concurrent installation with the colour Pypi package. [FEATURE]: Implement support for concurrent installation with vaab/colour Pypi package. Nov 8, 2023
@Voultapher
Copy link
Author

Please let me know once you release a new version that contains the new feature.

@KelSolaar
Copy link
Member

In December sometimes ideally, with Python 3.12 compat.

@KelSolaar KelSolaar added this to the v0.4.4 milestone Dec 15, 2023
@Voultapher
Copy link
Author

Voultapher commented Dec 18, 2023

After having tried it out, I can confirm it works. However there are some aspects that are not ideal.

  1. It's an explicit opt-in, so being spammed with dozens of lines of warnings every time is really not great
    image

  2. Usually in Python you can set the env variables directly in the import code e.g.:

import os
os.environ["COLOUR_SCIENCE__COLOUR__IMPORT_VAAB_COLOUR"] = "True"


# Has to come first, because it re-imports vaab/colour symbols needed by manim.
import colour

from manim import *

However this does not work here. Because the manim tool imports manim out of control of the user, which forces this property into end users of the tooling. Which I'd like to avoid.

@KelSolaar
Copy link
Member

KelSolaar commented Dec 18, 2023

Hi @Voultapher,

We discussed about this with the other Colour developers and it was preferred to have an opt-in behaviour, what we do here is dodgy as hell.

The warnings are admittedly annoying and I will add another environment variable to disable them in a future release.

Because the manim tool imports manim out of control of the user, which forces this property into end users of the tooling. Which I'd like to avoid.

Because it is opt-in, there won't be really other alternatives than either having the user to explicitly enable the injection or provide them wrappers that do so.

Cheers,

Thomas

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants