apertium / apertium-python

now you can even use apertium from python
GNU General Public License v3.0
31 stars 27 forks source link

Get rid of type: ignore #24

Closed sushain97 closed 5 years ago

sushain97 commented 6 years ago

There are a lot of them that I don't think we need.

orgh0 commented 6 years ago

@sushain97 I'm working on this in a separate branch. I've removed some of the comments but I'm not able to figure out what's wrong with the following function

def translate(self, text, mark_unknown=False, format=None, deformat='txt', reformat='txt'):
        # type: (Translator, str, bool, Optional[str], str, str) -> str
        if '%s-%s' % tuple(map(to_alpha3_code, [self.l1, self.l2])) in apertium.pairs:  # type: ignore
            pair = map(to_alpha3_code, [self.l1, self.l2])
        else:
            raise apertium.ModeNotInstalled()

        if pair is not None:
            l1, l2 = pair
            cmds = list(self._get_commands(l1, l2))
            unsafe_deformat, unsafe_reformat = self._get_format(format, deformat, reformat)
            deformater, reformater = self._validate_formatters(unsafe_deformat, unsafe_reformat)
            deformatted = self._get_deformat(str(deformater), text)
            output = execute(deformatted, cmds)
            result = self._get_reformat(str(reformater), output).strip()
            return result.decode()  # type: ignore

could you take a look ?

sushain97 commented 6 years ago

What's the error?

orgh0 commented 6 years ago
 vagrant@vagrant  apertium_linux  ~/Documents/apertium_code_base/apertium-python   typecheck-corrections ± mypy apertium --strict --any-exprs-report .mypy_coverage --ignore-missing-imports
apertium/translation/__init__.py:90: error: Not enough arguments for format string
apertium/translation/__init__.py:103: warning: Returning Any from function declared to return "str"
apertium/translation/__init__.py:103: error: "str" has no attribute "decode"; maybe "encode"?
orgh0 commented 6 years ago

90 and 103 are the lines with the ignore comment in them

sushain97 commented 6 years ago

If result is a str, then why are you calling .decode on it?

For L90, I suggest doing something like l1, l2 = map(to_alpha3_code, [self.l1, self.l2]) first.

orgh0 commented 6 years ago

@sushain97 L90 fixed :)

orgh0 commented 6 years ago

also, @sushain97

If result is a str, then why are you calling .decode on it?

type(result) = <class 'bytes'>
type(result.decode()) = <class 'str'>
sushain97 commented 6 years ago

Then, is the function that produces result typed badly?

On Tue, Jul 3, 2018, 11:28 PM Arghya Bhatttacharya notifications@github.com wrote:

also, @sushain97 https://github.com/sushain97

If result is a str, then why are you calling .decode on it?

type(result) = <class 'bytes'> type(result.decode()) = <class 'str'>

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apertium/apertium-python/issues/24#issuecomment-402376768, or mute the thread https://github.com/notifications/unsubscribe-auth/AEBEfttU90e63QyyBulagWTUAgmAyGNYks5uDGCAgaJpZM4U6f_1 .

orgh0 commented 6 years ago

@sushain97 Yes, that was the issue

singh-lokendra commented 5 years ago

Are there still type: ignore left that need to be removed?

sushain97 commented 5 years ago

You can search the repo on GitHub or use grep to determine if there any left.

On Fri, Jul 19, 2019, 2:20 AM Lokendra Singh notifications@github.com wrote:

Are there still type: ignore left that need to be removed?

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/apertium/apertium-python/issues/24?email_source=notifications&email_token=ABAEI7RD7THUDW2SZQBMXOLQAFTKNA5CNFSM4FHJ7722YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD2KZ6IY#issuecomment-513122083, or mute the thread https://github.com/notifications/unsubscribe-auth/ABAEI7XHPCO3T7WFFSQ4CWDQAFTKNANCNFSM4FHJ772Q .

singh-lokendra commented 5 years ago

https://github.com/apertium/apertium-python/blob/master/apertium/translation/__init__.py