Skip to content

Fix two files that do not parse - #139

Open
Lumen951 wants to merge 1 commit into
google:masterfrom
Lumen951:fix-unparseable-files
Open

Lumen951 wants to merge 1 commit into
google:masterfrom
Lumen951:fix-unparseable-files

Conversation

@Lumen951

Copy link
Copy Markdown

Reading through the repository to follow the inference path end to end, I could not
get the package to import from a clean checkout. Two files on master do not compile,
and between them they make most of the package unimportable.

ffn/training/tracker.py

9af8e87 wrapped the font used to annotate the eval image summaries in a try/except,
but left the call in the try body commented out, so the block is empty and the
module no longer parses. import ffn.inference.inference raises IndentationError
because of it, through inference -> movement -> training.examples -> training.tracker.

The except clause catches (IOError, ValueError), which is the documented exception
contract of PIL.ImageFont.truetype (OSError if the font file cannot be read,
ValueError if the size is not a positive number), so the commented-out line was
loading a specific font file. No font file is present in the repository and its path
and size cannot be recovered from it, so I kept the fallback that the except branch
already used. ImageDraw.text calls ImageFont.load_default() when it is handed no
font, which is exactly what the except branch passed, so the rendered summaries are
unchanged: both variants produce a byte-identical PNG.

ffn/input/volume.py

8c7fcb6 renamed config.sampling.bbox to config.sampling.bounding_boxes, and in
sample_coordinates the rename was made without re-indenting the block below it,
leaving if config.sampling.bounding_boxes: with no body and a 4-space indent inside
an otherwise 2-space file. The second occurrence of the same check, further down the
file, was updated correctly.

Verification

With both changes import ffn.inference.inference succeeds, and
ffn.inference.run_inference becomes importable. Of the 62 modules in the package, 46
now import, against 38 before; the rest fail on their own missing optional
dependencies. ffn/training/tracker_test.py passes.

Both were found while reading through the code base and trying to bring the
package up so the inference path could be followed end to end:
`import ffn.inference.inference` failed with an IndentationError. Two files on
master do not compile, and between them they make most of the package
unimportable.

ffn/training/tracker.py: 9af8e87 wrapped the font used to annotate the eval
image summaries in a try/except, but the call in the try body was left
commented out, so the block ended up empty. Anything that imports this file
fails, which reaches the inference core through
inference -> movement -> training.examples -> training.tracker.

ffn/input/volume.py: 8c7fcb6 renamed config.sampling.bbox to
config.sampling.bounding_boxes without re-indenting the block, leaving the
`if` with no body.

For tracker.py, the except clause catches (IOError, ValueError), which is the
documented exception contract of PIL.ImageFont.truetype (OSError: the font
file could not be read; ValueError: the size is not positive), so the line
that was commented out loaded a specific font file. Neither that file nor its
size can be recovered from the repository, which ships no font, so the
fallback the except branch already used is kept. ImageDraw.text falls back to
the same load_default() when handed no font, so rendering is unchanged.
@google-cla

google-cla Bot commented Sep 25, 2026

Copy link
Copy Markdown

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant