Skip to content

stub for csv.writer can't accept subclass of csv.Dialect #3611

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.

Sign up for GitHub

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

Jump to bottom
Closed
Solumin opened this issue Jan 14, 2020 · 1 comment · Fixed by #3613
Closed

stub for csv.writer can't accept subclass of csv.Dialect #3611

Solumin opened this issue Jan 14, 2020 · 1 comment · Fixed by #3613
Labels
stubs: false positive Type checkers report false errors

Comments

@Solumin
Copy link
Contributor

Solumin commented Jan 14, 2020

csv.writer is an import of _csv.writer. #3581 changed _csv.writer to take dialect: Union[_csv.Dialect, str]. This is correct, but now csv.Dialect -- which is distinct from _csv.Dialect -- is no longer a valid argument type for csv.writer.

For example, if someone was to define their own class MyDialect(csv.Dialect), a type checker would be correct to raise a "wrong argument type" error for a call like csv.writer(some_file, dialect=MyDialect()).

As far as I can tell, the stub is technically correct, but _csv.writer should be able to take subclasses of csv.writer. (Or at least I assume that's the intention. Notably, _csv.Dialect has a strict attribute that csv.Dialect lacks.)
@srittau srittau added the stubs: false positive Type checkers report false errors label Jan 14, 2020
@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Jan 14, 2020 •
edited
Loading

Thanks for finding!

It's confusing to me that there is csv.Dialect and _csv.Dialect in the first place. @JelleZijlstra suggested this was for performance reasons. Given that all the CPython code I can see seems to duck type around dialects, maybe the right thing to do is to just re-export _csv.Dialect in csv.

Seems like strict is documented for csv.Dialect, so maybe it's a bug in CPython that it's not mentioned in csv.py? (https://docs.python.org/3/library/csv.html#csv.Dialect.strict)

hauntsaninja pushed a commit to hauntsaninja/typeshed that referenced this issue Jan 14, 2020
csv: re-export Dialect from _csv
81297f4 
Resolves python#3611

Tested with mypy on:
```
f = open("asdf.csv", "w")

csv.writer(f)
csv.writer(f, dialect=csv.excel)
csv.writer(f, dialect=csv.excel())

csv.DictReader(f)
csv.DictReader(f, dialect=csv.excel)
csv.DictReader(f, dialect=csv.excel())

class CustomDialect(csv.Dialect):
    delimiter = "%"

csv.writer(f, dialect=CustomDialect)
csv.writer(f, dialect=CustomDialect())
```
hauntsaninja pushed a commit to hauntsaninja/typeshed that referenced this issue Jan 14, 2020
csv: re-export Dialect from _csv
1c67fc0 
Resolves python#3611

Tested with mypy on:
```
f = open("asdf.csv", "w")

csv.writer(f)
csv.writer(f, dialect=csv.excel)
csv.writer(f, dialect=csv.excel())

csv.DictReader(f)
csv.DictReader(f, dialect=csv.excel)
csv.DictReader(f, dialect=csv.excel())

class CustomDialect(csv.Dialect):
    delimiter = "%"

csv.writer(f, dialect=CustomDialect)
csv.writer(f, dialect=CustomDialect())
```
@hauntsaninja hauntsaninja mentioned this issue Jan 14, 2020
Merged
JelleZijlstra pushed a commit that referenced this issue Jan 23, 2020
@hauntsaninja @JelleZijlstra
csv: re-export Dialect from _csv (#3613)
b4da2b6 
Resolves #3611

Tested with mypy on:
```
f = open("asdf.csv", "w")

csv.writer(f)
csv.writer(f, dialect=csv.excel)
csv.writer(f, dialect=csv.excel())

csv.DictReader(f)
csv.DictReader(f, dialect=csv.excel)
csv.DictReader(f, dialect=csv.excel())

class CustomDialect(csv.Dialect):
    delimiter = "%"

csv.writer(f, dialect=CustomDialect)
csv.writer(f, dialect=CustomDialect())
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
stubs: false positive Type checkers report false errors
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

csv: re-export Dialect from _csv hauntsaninja/typeshed
3 participants
@srittau @Solumin @hauntsaninja