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

Shorten selector representations using an ellipsis #141

Merged
merged 5 commits into from
Jul 11, 2019

Conversation

Gallaecio
Copy link
Member

@Gallaecio Gallaecio commented Jul 10, 2019

Fork of #95 based on the feedback there by @kmike that never got an answer.

Fixes #91.

My Unicode heart asked me for using a single ellipsis character (…) in Python 3, but I guess having a consistent behavior across Python versions is preferred 🙂

I used a regular test function instead of doctests to test a wide array of scenarios without making the documentation too noisy.

I feel like the implementation is overkill. I blame the tests.

@codecov
Copy link

codecov bot commented Jul 10, 2019

Codecov Report

Merging #141 into master will increase coverage by 0.01%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #141      +/-   ##
==========================================
+ Coverage   99.63%   99.64%   +0.01%     
==========================================
  Files           5        5              
  Lines         271      279       +8     
  Branches       48       51       +3     
==========================================
+ Hits          270      278       +8     
  Misses          1        1
Impacted Files Coverage Δ
parsel/utils.py 100% <ø> (ø) ⬆️
parsel/selector.py 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1327e0d...63fe189. Read the comment docs.

@Gallaecio Gallaecio changed the title Selector data preview Shorten selector representations using an ellipsis Jul 10, 2019
parsel/utils.py Outdated Show resolved Hide resolved
@kmike
Copy link
Member

kmike commented Jul 11, 2019

Looks good, thanks @Gallaecio for finishing it, @harshasrinivas for the initial implementation and @eliasdorneles for reviews!

This PR has a risk of breaking some of the Scrapy tests, we'll need to take a look at it, probably when doing the Scrapy release.

@kmike kmike merged commit fafba41 into scrapy:master Jul 11, 2019
@Gallaecio
Copy link
Member Author

I’ve tried running tox -e py36 on scrapy after adding git+https://github.com/scrapy/parsel.git to its deps in tox.ini and tests passed, so we may get lucky 🙂

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

Successfully merging this pull request may close these issues.

Misleading "data=" in Selector representation
3 participants