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 regression in ocropus-gpageseg #251 #252

Merged
merged 2 commits into from Oct 17, 2017

Conversation

Projects
None yet
6 participants
@jze
Contributor

jze commented Oct 16, 2017

I looks as if pylab.find and np.where work slightly different.

@amitdo

This comment has been minimized.

Show comment
Hide comment
Contributor

amitdo commented Oct 16, 2017

@QuLogic

This comment has been minimized.

Show comment
Hide comment
@QuLogic

QuLogic Oct 16, 2017

Contributor

Please do not start using the pylab interface again.

Contributor

QuLogic commented Oct 16, 2017

Please do not start using the pylab interface again.

@kba

This comment has been minimized.

Show comment
Hide comment
@kba

kba Oct 16, 2017

Collaborator

Thanks for reporting and sorry for the regression.

IIUC, pylab.find does this:

def find(condition):
    "Return the indices where ravel(condition) is true"
    res, = np.nonzero(np.ravel(condition))
    return res

Does s,np.find/np.nonzero(np.ravel, fix the issue?

Collaborator

kba commented Oct 16, 2017

Thanks for reporting and sorry for the regression.

IIUC, pylab.find does this:

def find(condition):
    "Return the indices where ravel(condition) is true"
    res, = np.nonzero(np.ravel(condition))
    return res

Does s,np.find/np.nonzero(np.ravel, fix the issue?

@jze

This comment has been minimized.

Show comment
Hide comment
@jze

jze Oct 16, 2017

Contributor

Yes, it also works without pylab.

Contributor

jze commented Oct 16, 2017

Yes, it also works without pylab.

@kba

This comment has been minimized.

Show comment
Hide comment
@kba

kba Oct 16, 2017

Collaborator

LGTM, thank you.

@zuphilip In a154e63 this was the only place where pylab.find was replaced by np.where, right?

@jze If you provide the source images and CLI invocation, we could add a test to prevent this kind of error in the future

Collaborator

kba commented Oct 16, 2017

LGTM, thank you.

@zuphilip In a154e63 this was the only place where pylab.find was replaced by np.where, right?

@jze If you provide the source images and CLI invocation, we could add a test to prevent this kind of error in the future

@stweil

This comment has been minimized.

Show comment
Hide comment
@stweil

stweil Oct 16, 2017

Contributor

In a154e63 this was the only place where pylab.find was replaced by np.where, right?

Yes.

Contributor

stweil commented Oct 16, 2017

In a154e63 this was the only place where pylab.find was replaced by np.where, right?

Yes.

@zuphilip zuphilip changed the title from fixed https://github.com/tmbdev/ocropy/issues/251 to Fix regression in ocropus-gpageseg #251 Oct 16, 2017

@zuphilip zuphilip merged commit ebd8235 into tmbdev:master Oct 17, 2017

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@zuphilip

This comment has been minimized.

Show comment
Hide comment
@zuphilip

zuphilip Oct 17, 2017

Collaborator

Merged. Thank you very much @jze for the work and everybody here for the discussion!

Collaborator

zuphilip commented Oct 17, 2017

Merged. Thank you very much @jze for the work and everybody here for the discussion!

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