googlefonts / fontdiffenator

Font comparison tool
Apache License 2.0
81 stars 13 forks source link

Consolidate gftools/qa and fonbakery-dashboard worker startup boilerplate #54

Open graphicore opened 5 years ago

graphicore commented 5 years ago

When I implemented the diffenator worker for Font Bakery Dashboard I copied a lot of the gftools-qa.py scripts diffenator startup boilerplate.

Ideally the startup boilerplate should ship with diffenator, but also with some smaller changes, so that e.g. the Font Bakery Dashboard can launch parallel easily.

I'm going to describe what I have in mind in a following comment.


The duplicated code in question:

https://github.com/googlefonts/fontbakery-dashboard/blob/ee14f42425912f36b97461c4a5759ca98a343fa1/containers/base/python/worker/diffenator.py#L29-L122

graphicore commented 5 years ago

I'd like to have the following moved to diffenator (maybe in __init__.py?) or another more centrally shared module if it makes more sense:

directly from gftools-qa.py without changes:

changed from gftools-qa.py:

get_matching_fonts

# NEW
# 1. make a generator from `on_each_matching_font`

def get_matching_fonts(fonts_before, fonts_after):
    fonts_before_ttfonts = [TTFont(f) for f in fonts_before]
    fonts_after_ttfonts = [TTFont(f) for f in fonts_after]
    fonts_before_h = font_instances(fonts_before_ttfonts)
    fonts_after_h = font_instances(fonts_after_ttfonts)
    shared = set(fonts_before_h.keys()) & set(fonts_after_h.keys())
    if not shared:
        raise DiffenatorPreparationError(("Cannot find matching fonts. "
                         "Are font filenames the same?"))
    logger.info('Found %s comparable font instances.',  len(shared))
    for style in shared:
        yield (style,  fonts_before_h[style], fonts_after_h[style])

on_each_matching_font


# CHANGES
# 1. use new ` get_matching_fonts` generator
# for compatibility in gftools-qa (if interesting):

def on_each_matching_font(func):
    def func_wrapper(logger, fonts_before, fonts_after, out, *args, **kwargs):
        for (style, font_before, font_after) in\
                            get_matching_fonts(fonts_before, fonts_after):
            out_for_font = os.path.join(out, style)
            func(font_before, font_after, out_for_font, *args, **kwargs)

    return func_wrapper

run_diffenator_single

# NEW from decorated `run_diffenator`
# CHANGES
# 1. remove decorator, want to call this directly
#        BUT: If decorator with just one font on each input would work,
#        it would be fine. There's just too much magic happening.
# 2. add a default for threshold
# 3. added logger: should be the diffenator module logger,
#    though, it's debug output is enormously verbose!

def run_diffenator_single(font_before, font_after, out, thresholds=DIFFENATOR_THRESHOLDS['normal']):
    logger.debug('run_diffenator with fonts before: %s after: %s'
                                              , font_before, font_after)

    font_before = DFont(font_before)
    font_after = DFont(font_after)

    if font_after.is_variable and not font_before.is_variable:
        font_after.set_variations_from_static(font_before)

    elif not font_after.is_variable and font_before.is_variable:

 font_before.set_variations_from_static(font_after)

    elif font_after.is_variable and font_before.is_variable:
        # TODO get wdth and slnt axis vals
        variations = {"wght": font_before.ttfont["OS/2"].usWeightClass}
        font_after.set_variations(variations)
        font_before.set_variations(variations)

    diff = DiffFonts(font_before, font_after, settings=thresholds)
    diff.to_gifs(dst=out)
    diff.to_txt(20, os.path.join(out, "report.txt"))
    diff.to_md(20, os.path.join(out, "report.md"))
    diff.to_html(20, os.path.join(out, "report.html"), image_dir=".")

run_diffenator

# CHANGES
# 1, implement using the generator directly
# maybe nice to have, but I'd be fine to have these four lines in the fb-dashboard
# by myself

def run_diffenator(fonts_before, fonts_after, out, *args, **kwargs):        
    for (style, font_before, font_after) in get_matching_fonts(fonts_before, fonts_after):
        out_for_font = os.path.join(out, style)
        run_diffenator_single(font_before, font_after, out_for_font,
                                                            *args, **kwargs)

# OR use the decorator

run_diffenator = on_each_matching_font(run_diffenator_single)
graphicore commented 5 years ago

It's interesting, I added Diffbrowsers to Font Bakery Dashboard and it's very similar to Diffenator. especially get_matching_fonts should be shared between both. I think this proposal needs to evolve a bit more before action should be taken. I'll report back.