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

ValueError: Invalid config value: spinner=... when bundling script with PyInstaller #280

Open
mmatous opened this issue Sep 13, 2024 · 6 comments

Comments

@mmatous
Copy link

mmatous commented Sep 13, 2024

I encountered the same problem as described in #123 and applied the solution only to get a different error. I tried the CLI, specfile, and downgrade but the result is the same.

The error:

Traceback (most recent call last):
  File "ap.py", line 7, in <module>
    with alive_bar(n_tasks, spinner=spinner) as bar:
         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "alive_progress/core/progress.py", line 120, in alive_bar
ValueError: Invalid config value: spinner=<function spinner_controller.<locals>.inner_controller.<locals>.spinner_compiler_dispatcher_factory at 0x7f73bc554cc0>
Expected a custom factory or one of: ('classic', 'stars', 'twirl', 'twirls', 'horizontal', 'vertical', 'waves', 'waves2', 'waves3', 'dots', 'dots_waves', 'dots_waves2', 'it', 'ball_belt', 'balls_belt', 'triangles', 'brackets', 'bubbles', 'circles', 'squares', 'flowers', 'elements', 'loving', 'notes', 'notes2', 'arrow', 'arrows', 'arrows2', 'arrows_in', 'arrows_out', 'radioactive', 'boat', 'fish', 'fish2', 'fishes', 'crab', 'alive', 'wait', 'wait2', 'wait3', 'wait4', 'pulse')
[PYI-58196:ERROR] Failed to execute script 'ap' due to unhandled exception!

Example ap.py:

from alive_progress import alive_bar
from alive_progress.animations.spinners import bouncing_spinner_factory
import time

spinner = bouncing_spinner_factory(('A', 'B'), length=1)
n_tasks = 5
with alive_bar(n_tasks, spinner=spinner) as bar:
    for i in range(n_tasks):
        time.sleep(1)
        bar()

SW:
Gentoo Linux
Python 3.12.6
PyInstaller 6.10.0
alive-progress 3.1.5 (tried 1.6.2)

@rsalmei
Copy link
Owner

rsalmei commented Sep 14, 2024

Humm, I think it is the way I validate the spinner function:

func_file, _ = os.path.splitext(module_lookup.__file__)
if x.__code__.co_name == inner_name \
and os.path.splitext(x.__code__.co_filename)[0] == func_file:

I'm not sure what this pyinstaller makes with the Python source files, but I'm sure this second condition, which ensures the function came from one of my sources, failed.

Can you debug it? Just put in your file something like this, between spinner = and with alive_bar(:

    from ..animations import spinner_compiler
    print(spinner_compiler.__file__)
    print(spinner.__code__.co_name)
    print(spinner.__code__.co_filename)

@mmatous
Copy link
Author

mmatous commented Sep 14, 2024

plain script:

/home/mmatous/playground/aptest/venv/lib/python3.12/site-packages/alive_progress/animations/spinner_compiler.py
spinner_compiler_dispatcher_factory
/home/mmatous/playground/aptest/venv/lib/python3.12/site-packages/alive_progress/animations/spinner_compiler.py

bundled version:

/tmp/_MEIVE8Zos/alive_progress/animations/spinner_compiler.pyc
spinner_compiler_dispatcher_factory
alive_progress/animations/spinner_compiler.py

Getting rid of the second condition in ap's venv sources before bundling works. I'm OK with that workaround. Feel free to close the issue if you don't want to come up with a more permanent fix in AP.

@JoeJoeTV
Copy link

JoeJoeTV commented Oct 6, 2024

I am also facing this issue and have resorted to using the woraround proposed by @mmatous.
Similarly to their debug print output, the difference seems to be, that x.__code__.co_filename is a relative path in this case, while module_lookup.__file__ is an absolute path that includes the /tmp/....
Maybe this could be fixed by making sure that absolute/fully resolved paths are compared or, in case one path is relative, it could be checked that the suffixes match.
It seems like the x.__code__.co_filename path is relative the the alive_progress directroy, so one could get the absolute path to there and combine it:

parent_dir = os.path.dirname(os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
func_file, _ = os.path.splitext(module_lookup.__file__)
if x.__code__.co_name == inner_name \
        and os.path.splitext(os.path.join(parent_dir, x.__code__.co_filename))[0] == func_file:
    return x

(The 3 times application of dirname is maybe not the optimal setup, but it works in this case)

@mmatous
Copy link
Author

mmatous commented Oct 14, 2024

Imo we could do away with the entire check. Or check for mandatory attributes/methods/retval/accepted args if truly necessary. Who cares where the function came from? As long as it looks, swims and quacks like duck, it should be accepted and treated like it is one.

@rsalmei
Copy link
Owner

rsalmei commented Oct 15, 2024

I do. The spinner function is very internal and super hot, called up to 60 times per second, and has the potential to wreak havoc on the alive_progress performance and even working at all.
There's a reason I've protected it that way.

I think the permanent fix will be to change the == part, i.e., instead of testing whether the paths are exactly equal, just ensure they end with alive_progress' modules: alive_progress/animations/spinner_compiler.py. That should work.

@mmatous
Copy link
Author

mmatous commented Oct 16, 2024

I'll test it and send a PR with your proposed fix sometime next week.

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

3 participants