lk-geimfari / mimesis

Mimesis is a robust data generator for Python that can produce a wide range of fake data in multiple languages.
https://mimesis.name
MIT License
4.39k stars 330 forks source link

Type hinting #258

Closed sobolevn closed 6 years ago

sobolevn commented 6 years ago

I have started to use mypy. Aaaand it is not so good right now.

Libraries lack typing support. And I know that mimesis support only python3. Do you consider adding type hints?

There are some advantages:

There are some disadvantages:

lk-geimfari commented 6 years ago

Do you mean annotations?

Something like this:

def function(num: float) -> int:
    pass

Right?

sobolevn commented 6 years ago

Yes.

lk-geimfari commented 6 years ago

I thinks that we can add annotations. What about Python 3.4?

lk-geimfari commented 6 years ago

Seems like annotations was added in Python 3.0. And we lose nothing.

sobolevn commented 6 years ago

https://www.python.org/dev/peps/pep-0484/ Since 3.5

lk-geimfari commented 6 years ago

@sobolevn Soooo... What shall we do? Refusing to support Python 3.4 doesn't seem to be a good idea.

sobolevn commented 6 years ago

It actually is in some cases. Since upgrading from 3.4 to 3.5 is painless in 90% of situations.

lk-geimfari commented 6 years ago

@sobolevn Let's do this!

sobolevn commented 6 years ago

Please, note that variable annotations is not supported in 3.5. https://stackoverflow.com/questions/39971929/what-are-variable-annotations-in-python-3-6/39973133#39973133

So, we have to annotate methods and functions only.

lk-geimfari commented 6 years ago

@sobolevn I will add annotations for functions and methods. Can i hope for your help with situations which confuse me?

lk-geimfari commented 6 years ago

Example:

    def randints(self, amount: int = None, a: int = 1, b: int = 100) -> list:
        """Generate list of random integers.

        :param amount: Amount of elements.
        :param a: Minimum value of range.
        :param b: Maximum value of range.
        :return: List of random integers.
        """

        if not amount:
            amount = 3

        return [self.randint(a, b)
                for _ in range(amount)]

Is it right that amout: int = None. Is it should be integer default value or None is okay?

lk-geimfari commented 6 years ago

@sobolevn How to be if the function returns multiple types using? Should we use typing module?

sobolevn commented 6 years ago

In regular python without types I would say that None is perfectly fine. But in this particular situation it is possible to change amount=None to amount: int = 3. That what it does anyway, doesn't it?

About function returning multiple types. That's a code smell for me. It is error prone for many reasons:

So, consider not to do it.

lk-geimfari commented 6 years ago

@sobolevn Problem is in standard python library. json module returns both list and dict.

lk-geimfari commented 6 years ago

So, seems like i have found a solution.

sobolevn commented 6 years ago

With the provided solution there are a lot of uncovered cases, which are valid json:

emmeowzing commented 6 years ago

@lk-geimfari I've already mentioned this in #267, but if one is defaulting the argument to None, which is automatically pushed to type(None) or NoneType, then we should use typing.Optional[the_type]. In the example above, we'd have

def randints(self, amount: Optional[int] = None, a: int = 1, b: int = 100) -> list:
        """Generate list of random integers.

        :param amount: Amount of elements.
        :param a: Minimum value of range.
        :param b: Maximum value of range.
        :return: List of random integers.
        """

        if amount is None:
            amount = 3

        return [self.randint(a, b) for _ in range(amount)]

By the way, I believe I mistakenly reversed the logic of that predicate in my PR earlier :P

lk-geimfari commented 6 years ago

@bjd2385 I'll update code using Optional.

lk-geimfari commented 6 years ago

Sooo, it's done!