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

fixed backwards compatibility for scrapy.contrib.exporter.PythonItemExporter #1210

Merged
merged 1 commit into from May 5, 2015

Conversation

@kmike
Copy link
Member

@kmike kmike commented May 5, 2015

PythonItemExporter is not in __all__, so from scrapy.contrib.exporter import PythonItemExporter stopped working.

@kmike
Copy link
Member Author

@kmike kmike commented May 5, 2015

There shouldn't be other issues with __all__ and modules relocation.

@eliasdorneles
Copy link
Member

@eliasdorneles eliasdorneles commented May 5, 2015

cool! I've just been bitten by this ;)

@curita
Copy link
Member

@curita curita commented May 5, 2015

Thanks! Is there any reason why PythonItemExporter isn't in all btw?

@kmike
Copy link
Member Author

@kmike kmike commented May 5, 2015

I don't know; it wasn't there, so I didn't add it there.

IMHO exporters needs an overhaul anyways; PythonItemExporter doesn't handle all use cases, and unicode handling in exporters looks broken (see #1080). So maybe not adding PythonItemExporter to __all__, and thus discouraging its use is good for now. We should take a look at how other projects (e.g. django-rest-framework) handle it.

@curita
Copy link
Member

@curita curita commented May 5, 2015

Fair enough, let's merge it.

curita added a commit that referenced this pull request May 5, 2015
fixed backwards compatibility for scrapy.contrib.exporter.PythonItemExporter
@curita curita merged commit de9b234 into master May 5, 2015
1 of 2 checks passed
1 of 2 checks passed
continuous-integration/travis-ci/pr The Travis CI build is in progress
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
@dangra dangra deleted the relocations-fix branch May 5, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants