-
Notifications
You must be signed in to change notification settings - Fork 16
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 the default parameter to get_output_value #26
base: master
Are you sure you want to change the base?
Conversation
Codecov Report
@@ Coverage Diff @@
## master #26 +/- ##
==========================================
+ Coverage 98.02% 99.60% +1.58%
==========================================
Files 4 4
Lines 253 256 +3
==========================================
+ Hits 248 255 +7
+ Misses 5 1 -4
Continue to review full report at Codecov.
|
aaa5f7d
to
882882e
Compare
39fb0cc
to
afe7d06
Compare
Updated @Gallaecio |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I just have one consideration about the API.
In a dict
, get
will only return the default value if the key was not set.
Here, the default value is returned even if the field was set, but processing the value returned an "empty value" (''
, 0
, ...).
It looks good to me, though I think we should be clear as much as possible about this behavior.
Also, not sure if we need to add something to docs about it
You are right, we definitely need to update the documentation. |
Hey guys! Good job decoupling this from Scrapy!
I’ve been thinking about the possibility of adding a default value for the
get_output_value()
.So as an example, you could do:
and you would receive the value for
name
if it's available or'Foobar'
if it's not available.The implementation seems quite simple, but there are a lot of scenarios that I tried to reproduce in the tests.
Could you?
1. Let me know if you like the idea.
I decided to implement it because I frequently find something similar to:
2. Check the tests I added and see if it's what you expect or not.
If you see any missed case you can also let me know it.
Thanks in advance!