Skip to content

fix: accept a path-like tool_index_path, as weights already does - #110

Open
kevin9327 wants to merge 1 commit into
cactus-compute:mainfrom
kevin9327:fix/tool-index-path-like
Open

fix: accept a path-like tool_index_path, as weights already does#110
kevin9327 wants to merge 1 commit into
cactus-compute:mainfrom
kevin9327:fix/tool-index-path-like

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

Problem

Needle(tool_index_path=Path("tools.idx")) raises, but only when no tuned weights are passed:

AttributeError: 'PosixPath' object has no attribute 'encode'

The same argument is accepted a few lines further down. __init__ has two paths and they disagree:

self._weights = os.fspath(weights) if weights is not None else None          # path-like ok
...
self._tool_index_path = tool_index_path.encode("utf-8") if tool_index_path else None   # str only
...
    self._worker = FineTuneWorker(
        ..., os.fspath(tool_index_path) if tool_index_path else None, ...)   # path-like ok

So with weights= the agent takes a Path and without it the same call fails. weights itself
takes one either way, which is what a caller reasonably expects of the argument beside it, and
Path is the ordinary way to build a path next to the checkpoint you just wrote.

Fix

One line: os.fspath before the encode, matching weights directly above and the worker branch
directly below.

Tests

Two in tests/test_weights.py, on the existing engine fixture so no engine is needed:

  • test_tool_index_path_takes_a_path_object — a Path reaches the engine as its os.fspath bytes.
    Before this change it raises the AttributeError above.
  • test_tool_index_path_agrees_between_the_base_and_tuned_paths — the same Path is accepted by
    the base agent and by the worker-backed tuned agent, which is the asymmetry that made this easy to
    miss.

How I tested

Windows 11, Python 3.12.10, pip install -e ".[test,train]".
pytest -q -m "not slow" gives 66 passed, 9 skipped; the two new tests are the only change and
nothing else moves. The 9 skips are the six engine tests and the three worker tests that want a C
compiler.

test_lora.py, test_render.py, test_run.py, test_build.py and test_finetune.py are excluded
from that run: they need flax, whose orbax-checkpoint dependency will not install on Windows
because its paths exceed MAX_PATH. Nothing here touches them, and CI runs them on Ubuntu.

Signed-off-by: kevin9327 <kevin9327@users.noreply.github.com>
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