Language: consistency: retrieve dynamically from HTML where possible.

[jayaddison]

...and emit a Python warning to try to help us resolve cases where it isn't possible.

cc @rmdluo @mlduff because we've been discussing how to handle and emit warning messages.

[rmdluo]

The way you set the message here differs a little from the way we did it in my PR -- I think I put it at the top as a variable so that it could be reused in other functions within the module (even though it isn't). Since these messages seem to be used somewhat often between modules, I was thinking we could define another module with a few custom warning classes or messages. This would make the warning messages more standardized. We could also use them in testing https://docs.python.org/3/library/warnings.html#testing-warnings, which might make them a little less noisy during testing.

[mlduff]

@rmdluo I took some inspiration from your branch and went a step further, in this PR I have put it as a function in _utils. I'm currently investigating creating a custom warning category so that we could turn off those warnings during testing.

[rmdluo]

We can use catch_warnings to test that keys in the test json with a null value should raise a warning. This silences the warnings from the user side, but also tests to make sure that the warning is there. I have something implemented for this, but I'm not sure if we should make a separate pull request for this?

[rmdluo]

basic idea is this:

if expect[key]:
    self.assertEqual(
        expect[key],
        scraper_func(),
        msg=f"The actual value for .{key}() did not match the expected value.",
    )
else:
    with warnings.catch_warnings(record=True) as w:
        warnings.simplefilter("always")
        scraper_func()
        assert len(w) >= 1
        assert issubclass(w[-1].category, UserWarning) # can be adjusted to accomodate other warning types

[jayaddison]

Very much agreed with the idea of detecting this automatically using catch_warnings in the unit tests 👍

...however, value-is-null isn't exactly the condition I had in mind to detect; it's value-is-always-the-same (it's kinda a warning that the scraper isn't doing anything smart for that particular field, or really even returning anything from the page itself - it's simply returning a fixed value that is hopefully correct -- and worth double-checking -- for all recipes read by a scraper).

[rmdluo]

I guess for silencing the warnings, we could move catch_warnings to above the assert equal. For checking warning messages with non-null values, you might have to add more information to the test jsons to denote that the value stays the same.

[jayaddison]

I guess for silencing the warnings, we could move catch_warnings to above the assert equal.

👍 yep, ok - that sounds like a good idea to me

For checking warning messages with non-null values, you might have to add more information to the test jsons to denote that the value stays the same.

I'd prefer to encourage developers to provide this information in the scraper implementations. We'll probably forget to annotate some scrapers, but maybe with some clever linting/tooling we could, in future, detect methods that always return a fixed/literal value.

[jknndy]

This returns a lengthy output during a run of python -m unittest , is this the expected behavior?

(.venv) % python -m unittest
....................................../recipe_scrapers/bestrecipes.py:19: UserWarning: bestrecipes.com.au doesn't seem to provide language metadata in their HTML. Please let us know if it becomes available in a standard location, and then we can try to retrieve it dynamically.
  warnings.warn(msg)
................................................................................................................................................................................................................................./recipe_scrapers/potatorolls.py:52: UserWarning: potatorolls.com appears to write language metadata into HTML DOCTYPE declaration instead of the top-level 'html' element or elsewhere. Please let us know if it becomes available in a standard location, and then we can try to retrieve it dynamically.
  warnings.warn(msg)
........................................................................................................................................................................................................................................................................................................
----------------------------------------------------------------------
Ran 559 tests in 12.732s

OK

[jayaddison]

It's kinda-expected that it causes noise in the unittest output, yep. I'd wondered about that in another pull request, #1067, when discussing options for logging warnings for statically-defined fields (fields that only ever return a single value).

I had an idea and have pushed a commit (ff411e1) to implement it: we could ignore warnings from the scraper fields by default, but allow unit tests to be run with warning filters enabled, and add failures in the latter case when warnings occur.

Two drawbacks to that, though: it only works for Python3.11+, and also it means that the warnings are entirely hidden (no hints/reminders about them at all) when no Python warning options are configured.

[jayaddison]

Defaults

$ python -m unittest -k potato 
..
----------------------------------------------------------------------
Ran 2 tests in 0.172s

OK

Warnings enabled

$ python -Walways -m unittest -k potato 
FF
======================================================================
FAIL: tests/test_data/potatorolls.com/potatorolls_1.json (tests.RecipeTestCase.tests/test_data/potatorolls.com/potatorolls_1.json) [language]
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jka/Documents/reciperadar/recipe-scrapers/tests/__init__.py", line 110, in test_func
    self.fail(msg=msg)
AssertionError: .language() emitted warnings: UserWarning

======================================================================
FAIL: tests/test_data/potatorolls.com/potatorolls_2.json (tests.RecipeTestCase.tests/test_data/potatorolls.com/potatorolls_2.json) [language]
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/jka/Documents/reciperadar/recipe-scrapers/tests/__init__.py", line 110, in test_func
    self.fail(msg=msg)
AssertionError: .language() emitted warnings: UserWarning

----------------------------------------------------------------------
Ran 2 tests in 0.178s

FAILED (failures=2)