check_functions.py: fix error message to match expectation

[suvayu]

The expected value is in JSON normalised representation, however the error message is in Python representation. This led to confusing error messages, particularly for boolean values. JSON serialise using dump_db_value before formatting the error message to make this consistent.

Fixes spine-tools/Spine-Toolbox#1878

Any idea how to write a regression test?

Checklist before merging

• [ ] ~Documentation (also in Toolbox repo) is up-to-date~
• [ ] Release notes in Toolbox repo have been updated
• [x] Unit tests have been added/updated accordingly
• [x] Code has been formatted by black
• [x] Unit tests pass

[soininen]

Thanks a lot for the contribution!

Any idea how to write a regression test?

That would be a really nice addition! Unit tests reside in the tests/ directory. The directory structure therein mirrors the structure of spinedb_api/ and each file tests a specific class or module. I personally consider the per-class test files outdated and prefer to have a test file for each module (file) in spinedb_api/.

We don't seem to have tests for check_functions.py, so you might as well create one, i.e. tests/test_check_functions.py. You can use any of the other test files as a model/template. Note, that we're phasing out the :author: and :date: fields in the module-level docstrings.

The function that was changed here, _replace_values_with_list_references() is "private" (prepended by an underscore) so you probably shouldn't test that directly; Instead you could write a test for replace_parameter_values_with_list_references() that ultimately calls the correct function. You just need to feed the function with parameters that trigger the SpineIntegrityError exception. assertRaises() is your friend here.

[suvayu]

While working on the tests, I realised the method that parses values, unconditionally converts all numbers to floats. https://github.com/spine-tools/Spine-Database-API/blob/d3687464e4c629ac00518ecbb78c989b3c603ef2/spinedb_api/parameter_value.py#L166-L184

I'm curious, what is the reason behind this?

[suvayu]

Do you think a conversion to bool when we see 'TRUE' 'yes', etc would be good? Something like this:

def from_database(value):
    parsed = load_db_value(value)
    # ...
    if isinstance(parsed, str):
        return str_to_bool_maybe(parsed)

If str_to_bool_maybe can't find a match, it's a noop.

EDIT: looks like the right place to do this would be load_db_value, something like:

def lower_if_bool(value: str | bytes) -> str | bytes:
    value_lower = value.lower()
    if value_lower in ['true', 'false', b'true', b'false']:
        return value_lower
    else:
        return value

def load_db_value(db_value, ...):
    try:
        parsed = json.loads(lower_if_bool(db_value))
    except ...:
        ...

[soininen]

Do you think a conversion to bool when we see 'TRUE' 'yes', etc would be good? Something like this:

That would be the "proper" fix for the issue but should be rather done on user input side, i.e. in Spine Toolbox. to_database() and from_database() functions are meant to convert parameter values between their database representation and Python and not care about whatever gets pasted in Database editor. However, this is beyond the scope of this PR.