docker.legacy_build(pull="never") is treated as pull=True

[LewisGaul]

It seems that APIs that only accept booleans will be interpreted as a boolean value even if another type is given. This is particularly confusing in the case of docker.legacy_build(pull: bool), which is subtly different to docker.run(pull: str). Would it be worth doing some arg type validation, at least in the Command.add_flag() method?

[gabrieldemarmiesse]

Yes I think that would be helpful. I would be happy to get such a PR :)

EDIT: Maybe we should consider using beartype for this. That would avoid some complexity in our arguments. Maybe we should decorate our public api with this?

[LewisGaul]

Maybe we should consider using beartype for this. That would avoid some complexity in our arguments. Maybe we should decorate our public api with this?

That sounds like an interesting idea, I hadn't come across beartype before - what enigmatic docs!

[kashyab12]

EDIT: Maybe we should consider using beartype for this. That would avoid some complexity in our arguments. Maybe we should decorate our public api with this?

@gabrieldemarmiesse would the following suffice?

from beartype import beartype
...
    @beartype
    def add_flag(self, name: str, value: bool):
        if value:
            self.append(name)

I asserted that beartype catches these issues at runtime, so this should solve str types being passed. Moreover, the if value check can also removed as a result of this.

[gabrieldemarmiesse]

The issue with adding the beartype decorator used in non-public APIs is that the error messages won't be clean for the users. The error message will talk about a type mismatch in an internal function and it won't be obvious what caused this. I tend to think that decorating public apis functions with beartype would make error messages clearer from a user perspective.

[kashyab12]

That makes lots of sense, thanks for the explanation! I'm going to read more beartype docs before trying to work a fix. Appreciate the feedback!