Simplify creation of a required custom check

[joelostblom]

Currently, creating a custom check that is required involves the invocation of two different classes and the repetition of the message and path string for both:

import check_datapackage as cdp

license_check = cdp.CustomCheck(
    type="only-mit",
    jsonpath="$.licenses[*].name",
    message="Data Packages may only be licensed under MIT.",
    check=lambda license_name: license_name == "mit",
)
license_required = cdp.RequiredCheck(
    jsonpath="$.licenses[*].name",
    message="Data Packages may only be licensed under MIT.",
)

package_properties = cdp.example_package_properties()
config = cdp.Config(
    extensions=cdp.Extensions(
        custom_checks=[license_check], required_checks=[license_required]
    )
)
cdp.check(properties=package_properties, config=config, error=True)

We could of course assign the message and path strings to variables beforehand, but it is still a bit clunky. I saw that the 'required' type is explicitly disabled for the custom class, so I take it as there was some issue, but maybe this is still something to do here to make the workflow easier.

[lwjohnst86]

Hmm, I'm not sure I understand. What do you mean? The two checks are doing two different things. One makes it required and the other is making it required to use MIT. At least in this case, only the first is necessary.

In the RequiredCheck() the message would be more accurately stated "Data Packages must have a license"

[lwjohnst86]

Or you mean, if we want it to be both required and it's custom? You are right that there were lots of issues with it being type="required" in the CustomCheck(), hence why we split it out.

I think in this case, it's fine to have them as two separate configs. @martonvago might have other thoughts though.

[lwjohnst86]

Here's the issue with some of our discussions around this: #120

[martonvago]

Hmm, I also see those as two separate checks.

And it's not necessarily the case that when you define a custom check for a property, you want the check to fail if the property is missing. Maybe you only want to run the check if the property is there. Currently, CustomCheck calls check only if the property exists. If we called check for missing properties too, it would be the user's responsibility to expect a None value as a potential input to check. I think it's a nicer user experience if you don't have to account for this.

The other issue with required checks was that they don't make sense for all JSON paths (what does it mean to require $.resources[3]?), whereas custom checks do.

[lwjohnst86]

(But definitely agree, that it's a lot of code for adding configs. I think it makes more sense when we convert this into a CLI, where we won't expect users to necessarily work with the Python code directly.)