Type fixes

[DeviousStoat]

This adds a typing fix:

• uniq_by iteratee return type had wrong type, it can be Any to be able to get unique list based on some sub values of the elements

[DeviousStoat]

coveralls seems to have troubles processing the output in CI: coveralls.exception.CoverallsException: Could not submit coverage: 422 Client Error: Unprocessable Entity for url: https://coveralls.io/api/v1/jobs

This might be the answer: https://github.com/TheKevJames/coveralls-python/issues/251 But it seems that it is set: COVERALLS_SERVICE_NAME: github

[dgilland]

coveralls seems to have troubles processing the output in CI: coveralls.exception.CoverallsException: Could not submit coverage: 422 Client Error: Unprocessable Entity for url: https://coveralls.io/api/v1/jobs

This might be the answer: TheKevJames/coveralls-python#251 But it seems that it is set: COVERALLS_SERVICE_NAME: github

I tried fixing this with the suggestions in that issue and others but both --service=github and --service=github-actions are failing with the same 422 error. Some users reported that deleting the repo in coveralls.io and then re-adding it fixed things but I'm reluctant to do that since it would delete all historic data.

For now, I've disabled coveralls in GHA. This only started happening recently so hopefully it gets resolved by the coveralls teams since it doesn't seem like there's anything wrong with the GHA integration.

If you rebase or merge the latest in develop, then that should unblock this PR.

[DeviousStoat]

Thank you! Yep, looks good now

[dgilland]

• map_, flat_map, flat_map_deep, flat_map_depth, find and filter_ all can accept an optional collection as argument. This is very useful, especially in chaining expressions.

Was not having the None type causing some kind of issue?

Generally speaking, most of the functions that use pydash.helpers.iterator() can accept None as an argument, but I'm not sure if that's something that should be added everywhere.

[DeviousStoat]

It is not a big issue but yeah it is not right either. So for example doing something like this works fine at runtime:

import typing as t

import pydash

def process_list(my_list: t.Union[t.List[int], None]) -> t.List[int]:
    return pydash.map_(my_list, lambda x: x + 1)

But with current version we would get this with mypy:

error: Argument 1 to "map_" has incompatible type "Optional[List[int]]"; expected "Iterable[int]"  [arg-type]

This PR allows this now.

But actually I am not sure it is a good idea now. I feel it is a bit weird to try to process a None with list functions. I feel like we should encourage handling the None before doing any processing on the list, and don't let the library handle the None for you if you can as the main point of the functions it to be used on iterables.

import typing as t

import pydash

def process_list(my_list: t.Union[t.List[int], None]) -> t.List[int]:
    if my_list is None:
        return []

    return pydash.map_(my_list, lambda x: x + 1)

I feel like this is nicer and more explicit of what's going on, and if we can encourage this type of code, it might be better. What do you think?

[dgilland]

I feel like we should encourage handling the None before doing any processing on the list

Agree.

don't let the library handle the None for you if you can as the main point of the functions it to be used on iterables

If I was creating pydash from scratch today, then I don't think I'd have made it so permissible with input types, but since the None handling is already there, I'm inclined to leave it.

I feel like this is nicer and more explicit of what's going on, and if we can encourage this type of code, it might be better. What do you think?

Agree again that this kind of data validation should generally be handled before calling into pydash. But as mentioned above, I don't think it's feasible at this point to remove how pydash handles None, but we can encourage the "proper" usage through the type hints.

[DeviousStoat]

Perfect! I removed the concerned commits. This PR now just fixes uniq_by iteratee's return type which is a real problem.

[dgilland]

I wonder if it would help if we extended the type hints to helper functions. For example, this issue with uniq_by might have been caught if iterunique was typed since it then uses pydash.iteratee() which I think would have complained about the typing on iteratee in uniq_by.

Might be worth checking out other similar *_by functions like difference_by, intersection_by, pull_all_by, etc.

[DeviousStoat]

Damn you are right, they are all like that. No idea why I set them like that. I will make another PR to fix them too.

I wonder if it would help if we extended the type hints to helper functions. For example, this issue with uniq_by might have been caught if iterunique was typed since it then uses pydash.iteratee() which I think would have complained about the typing on iteratee in uniq_by.

Yeah it might probably help sometimes. But some of these functions are really hard to type. I guess I will try to take the time some day.

[dgilland]

But some of these functions are really hard to type.

If it's too painful then might not be worth it. Maybe a manual verification would be enough since types are unlikely to change.