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

Incorrect initialization of csviter in scrapy.spiders.feed.CSVFeedSpider #5391

Closed
dchaplinsky opened this issue Feb 5, 2022 · 0 comments · Fixed by #5394
Closed

Incorrect initialization of csviter in scrapy.spiders.feed.CSVFeedSpider #5391

dchaplinsky opened this issue Feb 5, 2022 · 0 comments · Fixed by #5394

Comments

@dchaplinsky
Copy link

Description

According to the master branch, scrapy.spiders.feed.CSVFeedSpider initializes csviter like this:

for row in csviter(response, self.delimiter, self.headers, self.quotechar):

https://github.com/scrapy/scrapy/blob/master/scrapy/spiders/feed.py#L126

But csviter definition says:
def csviter(obj, delimiter=None, headers=None, encoding=None, quotechar=None):
https://github.com/scrapy/scrapy/blob/master/scrapy/utils/iterators.py#L96

So effectively it passed quotechar instead of encoding

Steps to Reproduce

  1. Check the first link
  2. Check the second link
  3. [and so on...]

Expected behavior: CSVFeedSpider has a separate setting for the encoding or specify param names when calling csviter

Actual behavior: Mess

Reproduces how often: 100%

Versions

Scrapy       : 2.5.0
lxml         : 4.6.3.0
libxml2      : 2.9.10
cssselect    : 1.1.0
parsel       : 1.6.0
w3lib        : 1.22.0
Twisted      : 21.2.0
Python       : 3.9.8 (main, Nov 10 2021, 09:21:22) - [Clang 13.0.0 (clang-1300.0.29.3)]
pyOpenSSL    : 20.0.1 (OpenSSL 1.1.1k  25 Mar 2021)
cryptography : 3.4.7
Platform     : macOS-11.6-x86_64-i386-64bit

Additional context

Please fix it :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants