-
Notifications
You must be signed in to change notification settings - Fork 28
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
Update test expectations for a future where web_poet.pages.is_injectable(type(None)) is False #218
base: master
Are you sure you want to change the base?
Conversation
class OptionalAndUnionPage(WebPage): | ||
class OptionalAndUnionPageNew(WebPage): | ||
breadcrumbs: BreadcrumbsExtraction | ||
opt_check_1: Optional[BreadcrumbsExtraction] |
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.
Just in case, could you please add a test which checks that in case of Optional[something]
things don't break if something
is not available? There are similar tests in test_optional_and_unions_old, but not exactly that.
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.
Great catch. If it’s not injectable, that indeed causes an issue, I need to figure out a solution for that. It might have to do with attrs’ attribute default values not being recognized by andi, because I don’t think it happens for default values in regular callables (e.g. Scrapy callbacks).
not available
Did you mean not injectable, or should we also support scenarios where a bad type hint is provided? (e.g. var1: "UnexistingType"
or var2: "Optional[UnexistingType]"
)
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.
Tests updated, and issue addressed by scrapinghub/andi#33.
…table type even when there is a default value
To do:
Fix the issue.(the issue is in web-poet: Do not consider type(None) injectable web-poet#212)