Attention profile owners!

[simoncozens]

Hello @guidoferreyra @miguelsousa @josh-hadley @ollimeier and probably others I have not thought of...

I am currently working through a set of refactors to make the core of fontbakery simpler and more maintainable. (The number of people who understand and want to touch checkrunner.py or profile.py is vanishingly small.) As part of that, I am proposing/planning/working on a major refactor to the way that profiles are structured. Currently we have:

1. Profile definitions ("which checks to run") and check definitions ("how to run a check") mixed in the same file.
2. A strange and non-Pythonic "imports" system, profile "factories", "registration", and lots of other boilerplate bits that I don't quite understand.
3. Profile "sections" which aren't fully fledged/utilitized.
4. Log overrides as method calls.

It all works, but it's a bit clunky and in some cases cargo-cult - we end up copying and pasting bits from other people's profiles to make it all work.

What I'm proposing is:

1. Profiles and check implementations live in separate files. (Split checks from profiles #3188)
2. Profile definitions are fundamentally data. Overrides, imports, configurations, exclusions, etc. all live in a data file.
3. Checks organised into sections.

After the refactoring, the fontwerk.py profile now looks like this:

PROFILE = {
    "include_profiles": ["googlefonts"],
    "sections": {
        "Fontwerk Checks": [
            "com.fontwerk/check/no_mac_entries",
            "com.fontwerk/check/vendor_id",
            "com.fontwerk/check/weight_class_fvar",
            "com.fontwerk/check/inconsistencies_between_fvar_stat",
            "com.fontwerk/check/style_linking",
        ],
    },
    "exclude_checks": [
        # don't run these checks on the Fontwerk profile:
        "com.google.fonts/check/canonical_filename",
        "com.google.fonts/check/vendor_id",
        "com.google.fonts/check/fstype",
        "com.google.fonts/check/gasp",
        "com.google.fonts/check/name/description_max_length",
        "com.google.fonts/check/metadata/includes_production_subsets",
        "com.google.fonts/check/font_copyright",
        "com.google.fonts/check/version_bump",
        "com.google.fonts/check/production_glyphs_similarity",
        "com.google.fonts/check/name/line_breaks",
        "com.google.fonts/check/fontdata_namecheck",
        "com.google.fonts/check/meta/script_lang_tags",
        # The following check they may need some improvements
        # before we decide to include it:
        "com.google.fonts/check/family/italics_have_roman_counterparts",
    ],
    "configuration_defaults": {
        "com.google.fonts/check/file_size": {
            "WARN_SIZE": 1 * 1024 * 1024,
            "FAIL_SIZE": 9 * 1024 * 1024,
        }
    }
}

(With fontwerk-specific check definitions moved into fontbakery/checks/fontwerk.py.) Overriden results will look like this:

    "overrides": {
        "com.fontwerk/check/no_mac_entries": [
            {
                "code": "mac-names",
                "status": "WARN",
                "reason": "For TN, this is desired but not mandatory.",
            }
        ],
    ...

I know this is a substantial change but I hope it will make it easier for you to maintain your profiles as well.

I am also planning a later change to the way that "conditions" operate. These are currently a mix of two things: (a) a way to determine whether or not to skip a check, and (b) a function run on the font being tested with its result cached and injected into the check function as an argument. I am hoping to get rid of the second usage, because it's extremely non-obvious ("Whoa, how did that variable come from and why does it have the same name as a function?") and complicates the implementation. I haven't quite worked out what the replacement will be, but this is another heads up that there will (hopefully) be a lot of code movement in the way that checks are defined.

I'd welcome any questions, comments, requests, etc.

[simoncozens]

I am also planning a later change to the way that "conditions" operate. These are currently a mix of two things: (a) a way to determine whether or not to skip a check, and (b) a function run on the font being tested with its result cached and injected into the check function as an argument.

Or (c) sometimes we just use them as utility functions!

fontbakery/Lib/fontbakery/profiles/googlefonts.py

Lines 1074 to 1077 in 0b9860f

	from .googlefonts_conditions import get_glyphsets_fulfilled
	glyphsets_fulfilled = get_glyphsets_fulfilled(ttFont)

Like I said, it's a mess.

[madig]

What would a file with checks and conditions look like? Or do you have to work that out still?

[simoncozens]

That would look pretty much the same. The only (cosmetic) difference is that instead of

from fontbakery.callable import check, condition
from fontbakery.status import FAIL, PASS, WARN
from fontbakery.message import Message

you now do

from fontbakery.prelude import check, condition, FAIL, PASS, WARN, Message

[graphicore]

4. Log overrides as method calls.

In my model of Font Bakery, a check is conceptually a function not a method. That differentiation matters here, as a (pure) function when called with the same arguments produces the same result each time, but a method changes result based on the state of its instance. As a check override changes the function result, when called with the same arguments, it is in fact a different function.

[simoncozens]

Overrides are called on the profile object, though.

[graphicore]

Please elaborate what you mean with called on the profile object. I just looked at it and what I see is that profile.check_log_override creates a new check function that replaces the old check function. You can do that without the method call, it uses only public interfaces.

[simoncozens]

I just looked at it and what I see is that profile.check_log_override

That's an override method being called on the profile object.

I'm not sure the exact mechanics are entirely relevant. My point is that if we can do the same thing in a simpler way, that's a win. Having the profile as declarative data can achieve the same thing as code but, I believe, more simply.

[graphicore]

Whatever, then. For some syntactic sugar your operation is quite a slaughter though.