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

Dictionary lookup handler #61

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

share-with-me
Copy link
Contributor

No description provided.

self.pipeline_cmds[(l1, l2)] = translation.parseModeFile(mode_path)
return self.pipeline_cmds[(l1, l2)]

def getPairOrError(self, langpair):
Copy link
Member

Choose a reason for hiding this comment

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

Looks like this function is repeated? Maybe we should extract it BaseHandler?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Um, not all classes that extend BaseHandler use it. Do you suggest putting it at the BaseHandler level so that all the classes which extend it have this method?

Copy link
Member

Choose a reason for hiding this comment

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

Do you suggest putting it at the BaseHandler level so that all the classes which extend it have this method?

Yes.

@@ -343,6 +343,14 @@ def translateSimple(toTranslate, commands):
proc_in.stdout.close()
return translated.decode('utf-8')

@gen.coroutine
def translateNoFlush(toTranslate, commands):
Copy link
Member

Choose a reason for hiding this comment

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

This looks like the same as translateSimple?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes indeed. I've removed it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@sushain97 , the assert proc_in == proc_out line in translateSimple which is not present in translateNoFlush gives an assertion error.

@share-with-me
Copy link
Contributor Author

How do I resolve the current Travis build error? It fails with this message: Could not parse DTD /usr/share/lttoolbox/dix.dtd

@sushain97
Copy link
Member

Hm, I tried re-running the build to see if it was a transient error but that didn't help. @unhammer, thoughts?

@unhammer
Copy link
Member

unhammer commented Aug 19, 2017

https://travis-ci.org/goavki/apertium-apy/jobs/264033108 says it's flake8 issues, which travis build?

@sushain97
Copy link
Member

Ah, indeed! It looks like the transient DTD error has not subsisted after the re-build. @share-with-me check out the build output again. Thanks, @unhammer.

@sushain97 sushain97 force-pushed the master branch 2 times, most recently from b36ac97 to 51fb9c5 Compare January 3, 2022 10:56
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