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

Counter Attempt to Format the desc and unit Arguments #45

Closed
ZivRonen opened this issue Dec 9, 2021 · 6 comments
Closed

Counter Attempt to Format the desc and unit Arguments #45

ZivRonen opened this issue Dec 9, 2021 · 6 comments
Labels

Comments

@ZivRonen
Copy link

ZivRonen commented Dec 9, 2021

Describe the bug
Enlighten Counter try to format the description/unit fields - it will cause issues if '{' or '}' are present, in most cases raising ValueError.
This happen in both progress bar and counter modes.

To Reproduce

"""
Progress bar
"""
import enlighten
with enlighten.Counter(total=1, desc='A{') as pbar:
    pass
"""
Raw counter
"""
import enlighten
with enlighten.Counter(unit='B}') as pbar:
    pass

Environment (please complete the following information):

  • Enlighten Version: 1.10.1
  • OS and version: Windows 10
  • Console application: Cmd/PowerShell/Pycharm
  • Special Conditions: No

Additional context
N/A

@ZivRonen ZivRonen added the bug label Dec 9, 2021
@avylove
Copy link
Contributor

avylove commented Dec 9, 2021

This is because Enlighten uses str.format() internally to do variable substitution. To escape a curly brace, you need to double it.

with enlighten.Counter(total=1, desc='A{{') as pbar:
    pass

Let me know if that resolves your issue.
This should probably be in the FAQ.

@ZivRonen
Copy link
Author

ZivRonen commented Dec 9, 2021

Yes. I just thought it can be solved in the library, or at least be documented (if it was, I missed it).

Note: In my case the values are generated dynamically (os.walk of an unknown client) so I used:

with enlighten.Counter(total=1, desc=path.replace('{', '{{').replace('}', '}}')) as pbar:
   pass

which solved the issue for me.

Thanks for the quick response

P.S:
If you want, I can create a pull request to solve this more generally by perform format twice.
It will however slow the run a little, and will change the behavior as '{0}' will no longer be replaced in the description/unit (which I assume is unintentional, but I am not sure)

@avylove
Copy link
Contributor

avylove commented Dec 9, 2021

I just pushed a commit to take care of it. Let me know if it works for you. It would probably still be a good idea to add tests to protect against regression.

@ZivRonen
Copy link
Author

ZivRonen commented Dec 9, 2021

Checked it on my sample and it worked great!
Also checked the code and it looked fine to me, I don't see a real case that someone will use the new place holder by mistake :)

@avylove
Copy link
Contributor

avylove commented Dec 12, 2021

Released in 1.10.2

@avylove avylove closed this as completed Dec 12, 2021
@ZivRonen
Copy link
Author

Thank you!

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

No branches or pull requests

2 participants