Skip to content

Commit

Permalink
Moved check/whitespace_widths to Universal profile and added ration…
Browse files Browse the repository at this point in the history 
…ale text

com.google.fonts/check/whitespace_widths moved from `OpenType` profile to `Universal` profile.

And also:
* Added rationale text.
* Addressed a few flake8 concerns.
* Fixed flake8 setup: the setup.cfg file was overriding the flake8 setttings on tox.ini
(issue #3843)
  • Loading branch information
@felipesanches
felipesanches authored Aug 1, 2022
1 parent ba4fcd7 commit c74d907
Show file tree
Hide file tree
Showing 18 changed files with 193 additions and 180 deletions.
14 changes: 10 additions & 4 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,10 @@ A more detailed list of changes is available in the corresponding milestones for
- fixed bug on fontbakery_version check so that it now understands that v0.x.9 is older than v0.x.10 (issue #3813)
- Fix fontbakery.profiles.shared_conditions.*_*_coord functions so they work on Italic fonts (issue #3828, PR #3834)

### Migrations
#### To the `Universal` profile
- **[com.google.fonts/check/whitespace_widths]:** moved from `OpenType` profile. Also added rationale text. (issue #3843)

### Changes to existing checks
#### On the Universal Profile
- **[com.adobe.fonts/check/varfont/valid_default_instance_nameids]:** Relaxed the implementation to compare name values, not strictly IDs. (PR #3821)
Expand Down Expand Up @@ -165,8 +169,9 @@ A more detailed list of changes is available in the corresponding milestones for
- **[com.google.fonts/check/metadata/reserved_font_name]:** Added support for an RFN Exception allow-list, but it is kept empty for now while we review potential exceptions (issues #3589 and #3612)
- **[com.google.fonts/check/name/rfn]:** RFN Exception allow-list (same as above)

#### Migrations
- **[com.google.fonts/check/transformed_components]:** moved from `Google Fonts` profile to `Universal` profile. It is not strictly a Google Fonts related check as transformed components cause problems in various rendering environments. (issue #3588)
### Migrations
#### To the `Universal` profile
- **[com.google.fonts/check/transformed_components]:** moved from `Google Fonts` profile. It is not strictly a Google Fonts related check as transformed components cause problems in various rendering environments. (issue #3588)


## 0.8.6 (2022-Jan-29)
Expand Down Expand Up @@ -240,8 +245,9 @@ A more detailed list of changes is available in the corresponding milestones for
- **[com.google.fonts/check/contour_count]:** Do not expect ZWNJ and ZWJ glyphs to have zero contours. (issue #3487)
- **[com.google.fonts/check/metadata/can_render_samples]:** Ensure METADATA.pb is present before running check (issue googlefonts/Unified-Font-Repository#62)

#### Migrations
- **[com.google.fonts/check/contour_count]:** moved from `Google Fonts` profile to `Universal` profile. (issue #3491)
### Migrations
#### To the `Universal` profile
- **[com.google.fonts/check/contour_count]:** moved from `Google Fonts` profile. (issue #3491)


## 0.8.3 (2021-Oct-28)
Expand Down
99 changes: 53 additions & 46 deletions Lib/fontbakery/callable.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

from functools import wraps, update_wrapper


def cached_getter(func):
"""Decorate a property by executing it at instatiation time and cache the
result on the instance object."""
Expand All @@ -28,6 +29,7 @@ def wrapper(self):
return value
return wrapper


class FontbakeryCallable:
def __init__(self, func):
self._args = None
Expand All @@ -44,10 +46,10 @@ def __init__(self, func):
update_wrapper(self, func)

def __repr__(self):
return'<{}:{}>'.format(type(self).__name__,
getattr(self, 'id',
getattr(self, 'name',
super().__repr__() ))) # pylint: disable=consider-using-f-string
return '<{}:{}>'.format(type(self).__name__,
getattr(self, 'id',
getattr(self, 'name',
super().__repr__()))) # pylint: disable=consider-using-f-string

@property
@cached_getter
Expand All @@ -66,12 +68,12 @@ def mandatoryArgs(self):
inspect.Parameter.POSITIONAL_ONLY):
# has a default i.e. not mandatory or not positional of any kind

## Debugging message:
#print(f'[{param}]'
# f' {param.default is inspect.Parameter.empty}'
# f' param.kind: {param.kind}'
# f' param.default: {param.default}'
# f' BREAK')
# Debugging message:
# print(f'[{param}]'
# f' {param.default is inspect.Parameter.empty}'
# f' param.kind: {param.kind}'
# f' param.default: {param.default}'
# f' BREAK')
break
args.append(name)
return tuple(args)
Expand All @@ -91,12 +93,12 @@ def optionalArgs(self):
inspect.Parameter.POSITIONAL_ONLY):
# no more positional of any kind

## Debugging message:
#print(f'[{param}]'
# f'{param.default is inspect.Parameter.empty}'
# f' param.kind: {param.kind}'
# f' param.default: {param.default}'
# f' BREAK')
# Debugging message:
# print(f'[{param}]'
# f'{param.default is inspect.Parameter.empty}'
# f' param.kind: {param.kind}'
# f' param.default: {param.default}'
# f' BREAK')
break
args.append(name)
return tuple(args)
Expand Down Expand Up @@ -134,13 +136,14 @@ def get_doc_desc(func, description, documentation):

return description, documentation


class FontBakeryCondition(FontbakeryCallable):
def __init__(self,
func,
# id,
name = None, # very short text
description = None, # short text
documentation=None, # long text, markdown?
name=None, # very short text
description=None, # short text
documentation=None, # long text, markdown?
force=False):
super().__init__(func)
# self.id = id
Expand All @@ -150,31 +153,32 @@ def __init__(self,
documentation)
self.force = force


class FontBakeryCheck(FontbakeryCallable):
def __init__(self,
checkfunc,
id,
description=None, # short text, this is mandatory
description=None, # short text, this is mandatory
documentation=None,
name=None, # very short text
name=None, # very short text
conditions=None,
rationale=None, # long text explaining why this check is needed.
# Using markdown, perhaps?
proposal=None, # An URL to the original proposal for this check.
# This is typically a github issue or pull request.
severity=None, # numeric value from 1=min to 10=max, denoting check severity
configs=None, # items from config[self.id] to inject into the check's namespace.
misc_metadata=None, # Miscelaneous free-form metadata fields
# Some of them may be promoted to 1st-class metadata fields
# if they start being used by the check-runner.
# Below are a few candidates for that:

# affects=None, # A list of tuples each indicating Browser/OS/Application
# # and the affected versions range.
# example_failures=None, # A reference to some font or family that
# # originally failed due to the problems
# # that this check tries to detect and report.
):
rationale=None, # long text explaining why this check is needed.
# Using markdown, perhaps?
proposal=None, # An URL to the original proposal for this check.
# This is typically a github issue or pull request.
severity=None, # numeric value from 1=min to 10=max, denoting check severity
configs=None, # items from config[self.id] to inject into the check's namespace.
misc_metadata=None, # Miscelaneous free-form metadata fields
# Some of them may be promoted to 1st-class metadata fields
# if they start being used by the check-runner.
# Below are a few candidates for that:

# affects=None, # A list of tuples each indicating Browser/OS/Application
# # and the affected versions range.
# example_failures=None, # A reference to some font or family that
# # originally failed due to the problems
# # that this check tries to detect and report.
):
"""This is the base class for all checks. It will usually
not be used directly to create check instances, rather
decorators which are factories will init this class.
Expand Down Expand Up @@ -236,6 +240,7 @@ def __init__(self,
# def __str__(self):
# return self.id


def condition(*args, **kwds):
"""Check wrapper, a factory for FontBakeryCondition
Expand All @@ -252,6 +257,7 @@ def wrapper(func):
return FontBakeryCondition(func, *args, **kwds)
return wrapper


def check(*args, **kwds):
"""Check wrapper, a factory for FontBakeryCheck
Expand All @@ -262,17 +268,18 @@ def wrapper(checkfunc):
return FontBakeryCheck(checkfunc, *args, **kwds)
return wrapper


# ExpectedValue is not a callable, but it belongs next to check and condition
_NOT_SET = object() # used as a marker
_NOT_SET = object() # used as a marker
class FontBakeryExpectedValue:
def __init__(self,
name, # unique name in global namespace
description=None, # short text, this is mandatory
documentation=None, # markdown?
default=_NOT_SET, # because None can be a valid default
validator=None, # function, see the docstring of `def validate`
name, # unique name in global namespace
description=None, # short text, this is mandatory
documentation=None, # markdown?
default=_NOT_SET, # because None can be a valid default
validator=None, # function, see the docstring of `def validate`
force=False
):
):
self.name = name
self.description = description
self.documentation = documentation
Expand All @@ -281,7 +288,7 @@ def __init__(self,
self.force = force

def __repr__(self):
return'<{}:{}>'.format(type(self).__name__, self.name)
return '<{}:{}>'.format(type(self).__name__, self.name)

@property
def has_default(self):
Expand Down
4 changes: 2 additions & 2 deletions Lib/fontbakery/cli.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -48,8 +48,8 @@ def main():
sys.argv[0] += " " + subcommand
del sys.argv[1] # Make this indirection less visible for subcommands.
if (
subcommand_module.startswith("check_")
and subcommand_module[6:] in CLI_PROFILES
subcommand_module.startswith("check_") and
subcommand_module[6:] in CLI_PROFILES
):
run_profile_check(subcommand_module[6:])
else:
Expand Down
69 changes: 36 additions & 33 deletions Lib/fontbakery/multiproc.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,35 +16,34 @@
import queue

from fontbakery.reporters import FontbakeryReporter
from fontbakery.checkrunner import ( # NOQA
INFO
, WARN
, ERROR
, STARTCHECK
, SKIP
, PASS
, FAIL
, ENDCHECK
, END
, DEBUG
, CheckRunner
, session_protocol_generator
, drive_session_protocol
, get_profile_from_module_locator
)
from fontbakery.checkrunner import (INFO,
WARN,
ERROR,
STARTCHECK,
SKIP,
PASS,
FAIL,
ENDCHECK,
END,
DEBUG,
CheckRunner,
session_protocol_generator,
drive_session_protocol,
get_profile_from_module_locator)
from fontbakery.message import Message


################
# WORKER/CHILD #
################

name2status = {status.name:status for status in \
name2status = {status.name: status for status in \
(DEBUG, PASS, SKIP, INFO, WARN, FAIL, ERROR)}


# Similar to DashbordWorkerReporter of Font Bakery Dashboard.
class WorkerToQueueReporter(FontbakeryReporter):
def __init__(self, queue, profile, ticks_to_flush = None, **kwd):
def __init__(self, queue, profile, ticks_to_flush=None, **kwd):
super().__init__(**kwd)
self._queue = queue
self._profile = profile
Expand Down Expand Up @@ -124,11 +123,12 @@ def flush(self):
self._queue.put(tuple(self._collectedChecks.items()))
self._collectedChecks = None

#This is the inverse of the serialization in WorkerToQueueReporter

# This is the inverse of the serialization in WorkerToQueueReporter
def check_protocol_from_worker_data(profile, key_check_data):
key, check_data = key_check_data
identity = profile.deserialize_identity(key)
yield STARTCHECK, None, identity ## = event
yield STARTCHECK, None, identity # = event

for log in check_data['statuses']:
status = name2status[log['status']]
Expand All @@ -140,10 +140,11 @@ def check_protocol_from_worker_data(profile, key_check_data):
setattr(message, 'traceback', log['traceback'])
else:
message = log['message']
yield status, message, identity ### = event
yield status, message, identity # = event

status = name2status[check_data['result']]
yield ENDCHECK, status, identity ## = event
yield ENDCHECK, status, identity # = event


def _worker_jobs_generator(jobs_queue, profile, reporter):
while True:
Expand All @@ -161,19 +162,19 @@ def _worker_jobs_generator(jobs_queue, profile, reporter):
job = jobs_queue.get(True)
yield profile.deserialize_identity(job)

def multiprocessing_worker(jobs_queue, results_queue, profile_module_locator
, runner_kwds):

def multiprocessing_worker(jobs_queue, results_queue, profile_module_locator, runner_kwds):
profile = get_profile_from_module_locator(profile_module_locator)
runner = CheckRunner(profile, **runner_kwds)
reporter = WorkerToQueueReporter( results_queue
, profile=profile
, runner=runner
, ticks_to_flush=5
)
reporter = WorkerToQueueReporter(results_queue,
profile=profile,
runner=runner,
ticks_to_flush=5)

next_check_gen = _worker_jobs_generator(jobs_queue, profile, reporter)
runner.run_externally_controlled(reporter.receive, next_check_gen)


#####################
# DISPATCHER/PARENT #
#####################
Expand All @@ -188,6 +189,7 @@ def _results_generator(results_queue, len_results):
if count_results >= len_results:
return


@contextmanager
def _multiprocessing_checkrunner(jobs, process_count, *args):
jobs_queue = Queue()
Expand All @@ -209,21 +211,22 @@ def _multiprocessing_checkrunner(jobs, process_count, *args):
args=(jobs_queue, results_queue, *args))
processes.append(p)
p.start()
yield _results_generator(results_queue, len_jobs) # next_check_gen
yield _results_generator(results_queue, len_jobs) # next_check_gen
finally:
for p in processes:
p.terminate()
p.join()


def multiprocessing_runner(process_count, runner, runner_kwds):
# process_count is a positive int, never 0 at this point
assert process_count > 0
profile = runner.profile
joblist = list(profile.serialize_order(runner.order))

session_gen = session_protocol_generator(
partial(check_protocol_from_worker_data, profile),
runner.order)
session_gen = session_protocol_generator(partial(check_protocol_from_worker_data,
profile),
runner.order)
with _multiprocessing_checkrunner(joblist,
process_count,
profile.module_locator,
Expand Down
5 changes: 1 addition & 4 deletions Lib/fontbakery/profiles/adobefonts.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -111,10 +111,6 @@
"com.google.fonts/check/maxadvancewidth",
#
# =======================================
# From hmtx.py
"com.google.fonts/check/whitespace_widths",
#
# =======================================
# From kern.py
"com.google.fonts/check/kern_table",
#
Expand Down Expand Up @@ -200,6 +196,7 @@
"com.google.fonts/check/gpos7",
"com.adobe.fonts/check/freetype_rasterizer",
"com.adobe.fonts/check/sfnt_version",
"com.google.fonts/check/whitespace_widths",
}

CHECKS_IN_THIS_FILE = [
Expand Down
Loading

0 comments on commit c74d907

Please sign in to comment.