Rework ExtractionFilter to adept to boolean values

[MaxDall]

With #422, 'free_access' is computed correctly. Because not all publishers set this information correctly, I felt the need to rework the extraction filter again.

• I added a new parameter to Requires, skip_boolean which enables one to skip boolean values for evaluation
• There are now two predefined extraction filters: Requires, RequiresAll,
• RequiresAll is a name wrap for Requires() and propagates the skip_bool parameter to the user.

[addie9800]

I would suggest also updating the default value for the only_complete attribute in the crawl() method of crawler.py

[MaxDall]

I would suggest also updating the default value for the only_complete attribute in the crawl() method of crawler.py

Hmm, what would be the reasoning behind this? Maybe I'm missing something, but the behavior of Requires("title", "body", "publishing_date") doesn't change at all with this PR.

[addie9800]

Hmm, what would be the reasoning behind this? Maybe I'm missing something, but the behavior of Requires("title", "body", "publishing_date") doesn't change at all with this PR.

From what I see, the current default value is Requires("title", "body", "publishing_date", [skip_bool = False]). As far as I understood, the goal of this PR was to update the Requires() functionality to ignore the boolean values per default.

[MaxDall]

From what I see, the current default value is Requires("title", "body", "publishing_date", [skip_bool = False]). As far as I understood, the goal of this PR was to update the Requires() functionality to ignore the boolean values per default.

Yeah, that's right, but neither title, body nor publishing_date is a boolean value.

Update: Wait, maybe there is a misunderstanding here. Requires shouldn't ignore booleans per default so that Requires("free_access") actually requires the articles to have free_access=True. RequiresAll() should skip booleans per default.

[addie9800]

Yeah, that's right, but neither title, body nor publishing_date is a boolean value.

Update: Wait, maybe there is a misunderstanding here. Requires shouldn't ignore booleans per default so that Requires("free_access") actually requires the articles to have free_access=True. RequiresAll() should skip booleans per default.

OK, I think I see my mistake. So just to be sure: the expected behavior of Requires("title", "body", "publishing_date") is the same as Requires("title", "body", "publishing_date", "free_access", skip_bool = True)?

[MaxDall]

OK, I think I see my mistake. So just to be sure: the expected behavior of Requires("title", "body", "publishing_date") is the same as Requires("title", "body", "publishing_date", "free_access", skip_bool = True)?

No, Requires checks only passed attributes. So Requires("title", "body", "publishing_date") behaves differently than Requires("title", "body", "publishing_date", "free_access") in a way that the latter checks the extraction for the free_access attribute as well. skip_boolean only determines if boolean values are evaluated with bool or if it is checked if they are present in the extraction at all.

[addie9800]

OK, now I think I got the idea. I guess the name confused me a bit, perhaps we could change it to something along the lines of eval_bools or eval_bool_values? Also, should we mention this change in the docs?

[MaxDall]

OK, now I think I got the idea. I guess the name confused me a bit, perhaps we could change it to something along the lines of eval_bools or eval_bool_values? Also, should we mention this change in the docs?

That's a good point. I agree, skip_boolean seems quite bad 😅. Do you have any ideas? IMO eval_bools fits better but isn't quite there.

[addie9800]

Do you have any ideas?

Not really, but maybe something like filter_by_bool_value or require_existence

[addie9800]

👍