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

convert(ports=) is a hotspot of beginner issues #362

Closed
whitequark opened this issue Apr 21, 2020 · 1 comment · Fixed by #366
Closed

convert(ports=) is a hotspot of beginner issues #362

whitequark opened this issue Apr 21, 2020 · 1 comment · Fixed by #366
Milestone

Comments

@whitequark
Copy link
Member

whitequark commented Apr 21, 2020

We have a disproportionally large amount of questions from beginners that relate to the ports argument of convert() functions. We should add stricter typechecking to it (accepting either list or tuple, but nothing else, seems like a good idea), plus a "did you mean?" type diagnostic for passing a single signal in place of an iterable of ports.

@ewenmcneill
Copy link

ewenmcneill commented Apr 23, 2020

Having been through this (type checking to avoid mistakes) in other contexts, the other way to approach "acceptable type" would be "iterable, but not string-like" (strings are iterable in Python, but often not what you intended to give something that expects an iteratorable).

I'm unsure in this context whether it makes sense to be able to provide, eg, a generator/generator comprehension to the argument. But if it is then checking "iterable, but not string" with a suggestion of passing in a list or tuple (for obviously mistaken input) might be the most useful combination.

Ewen

whitequark pushed a commit that referenced this issue Apr 24, 2020
The `ports` argument to the `convert` functions is a frequent hotspot of
beginner issues. Check to make sure it is either a list or a tuple, and
give an appropriately helpful error message if not.

Fixes #362.
@whitequark whitequark added this to the 0.3 milestone May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging a pull request may close this issue.

2 participants