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

Fix for 'TypeError: float argument required, not str' #103

Merged
merged 1 commit into from
Oct 20, 2016

Conversation

jmorrey
Copy link
Contributor

@jmorrey jmorrey commented Jun 28, 2016

We were trying to cast "scale" to a string then print it as a float.

@zuphilip
Copy link
Collaborator

Here is an example for reproducing the error which is fixed here:

ocropus-gpageseg --minscale 12 --scale 10 tests/testpage.png
INFO:
INFO:  ########## /usr/local/bin/ocropus-gpageseg --minscale 12 --scale 10 tes
INFO:
INFO:  tests/testpage.png
INFO:  scale 10.0
Traceback (most recent call last):
  File "/usr/local/bin/ocropus-gpageseg", line 423, in safe_process1
    process1(job)
  File "/usr/local/bin/ocropus-gpageseg", line 373, in process1
    print_error("%s: scale (%g) less than --minscale; skipping\n"%(fname,str(scale)))
TypeError: float argument required, not str

Copy link
Collaborator

@zuphilip zuphilip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested this commit and it fixes the error.

@kba
Copy link
Collaborator

kba commented Oct 20, 2016

Why cast scale to str in the first place? The point of using "%g" % (123) or similar is to do explicit to-string conversion.

# Why this
print_error("%s: scale (%s) less than --minscale; skipping\n"%(fname,str(scale)))
# instead of this
print_error("%s: scale (%g) less than --minscale; skipping\n"%(fname,scale))

This happens in quite a few places:

ag -l 'print.*strc?\('
ocropus-nlbin
ocropus-gpageseg
ocropus-rpred
ocrolib/toplevel.py  

But that is a different issue and should be fixed in a new PR.

@kba kba merged commit 1a906e3 into ocropus-archive:master Oct 20, 2016
@zuphilip
Copy link
Collaborator

zuphilip commented Oct 23, 2016

Thank you very much, @jmorrey, for reporting and fixing the issue with this pull request!

(As you see this leads to more questions and more things to change, which we will now continue.)

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.

3 participants