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

Add .get() and .getall() aliases #81

Merged
merged 3 commits into from May 6, 2017
Merged

Add .get() and .getall() aliases #81

merged 3 commits into from May 6, 2017

Conversation

redapple
Copy link
Contributor

In quite a few projects, I've been using a .get() alias to .extract_first() as most of the time, getting the first match is what I want.
To me, .extract_first() feels a bit long to write (I'm probably getting lazy with age...)

For cases where I do need to loop on results, I added a .getall() alias for .extract() on .xpath() and .css() calls results.

I know there's been quite some discussion already to have .extract_first() in the first place, but I'm submitting my preference again.

@codecov-io
Copy link

codecov-io commented Apr 19, 2017

Codecov Report

Merging #81 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master    #81   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files           4      4           
  Lines         204    212    +8     
  Branches       35     36    +1     
=====================================
+ Hits          204    212    +8
Impacted Files Coverage Δ
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 75eb879...94ede59. Read the comment docs.

@eliasdorneles
Copy link
Member

+1, I really like this.

How do you feel about adding a getall in Selector class as well?
It's so annoying having to explain the differences between SelectorList and Selector.

@redapple
Copy link
Contributor Author

@eliasdorneles , interesting. would Selector.getall() return a single-element list?

@eliasdorneles
Copy link
Member

eliasdorneles commented Apr 19, 2017 via email

@redapple
Copy link
Contributor Author

redapple commented May 4, 2017

@eliasdorneles , done!

@eliasdorneles
Copy link
Member

Yesss!!
This is great, easier to predict which method to use (get always returns the first, and getall returns as many as possible for the given context), you don't need to think much before deciding which method to use.
Thanks @redapple !

@eliasdorneles eliasdorneles merged commit 2c87fe4 into scrapy:master May 6, 2017
@eliasdorneles
Copy link
Member

Hey @redapple @kmike !
I think this deserves a 1.2.0 release, I really like these new methods, and there is also #74 (new feature) and #82 (bugfix) since the last release.
What do you think?

@redapple
Copy link
Contributor Author

I'm all for it.

@redapple
Copy link
Contributor Author

It would be ncie to get this one fixed also: #84

@redapple redapple added this to the v1.2 milestone May 17, 2017
@kmike kmike mentioned this pull request Jun 22, 2018
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.

None yet

3 participants