[marta-sd]

Extended usage of --search-dir flag and incorporates env vars logic from #1264 by @gwarmstrong

[copy-pr-bot]

This pull request requires additional validation before any workflows can run on NVIDIA's runners.

Pull request vetters can view their responsibilities here.

Contributors can view more details about this message here.

[marta-sd]

/ok to test b32c6d0

[prokotg]

I know it's an inherited problem, but I'd suggest to move definitions from __init__.py to a dedicated module

[marta-sd]

this would be harder / uglier than it seems.

I can't move old things like PARENT_DIR plus I need to call _augment_sys_path in init. George suggests using component_search_roots in _augment_sys_path (comment) which I think makes sense.

So in principle we have a circular dependency between this new dedicated module and __init__.py. It's possible to avoid circular import error (I think), but I prefer the current solution is a lesser evil.

[prokotg]

Ok, let's consider this problem separately from this MR but I strongly discourage having anything than declaration, re-imports or variable definition in __init__

[gwarmstrong]

I think there's a few useful command here that are missing --search-dir, e.g., all of the following I think have valid use-cases, but aren't enabled here:

gym env resolve \
  --search-dir ../my-plugin \
  --config configs/my-config.yaml

gym dataset render \
  --search-dir ../my-plugin \
  --input data.jsonl \
  --prompt-config prompts/my-prompt.yaml \
  --output rendered.jsonl

gym env test \
  --search-dir ../my-plugin \
  --resources-server my-server

gym env packages \
  --search-dir ../my-plugin \
  --resources-server my-server

[gwarmstrong]

Could _augment_sys_path() use component_search_roots() in some way instead of rebuilding the root order separately? The current order looks correct, but file lookup and Python imports still get their ordering from different logic. Sharing the same function would make it less likely that they diverge later.

E.g., if we have:

/tmp/my-plugin/
└── benchmarks/
    └── aime24/
        ├── __init__.py
        ├── config.yaml
        └── prepare.py

When running with:

gym eval prepare --search-dir /tmp/my-plugin --benchmark aime24

Gym should load both:

/tmp/my-plugin/benchmarks/aime24/config.yaml
/tmp/my-plugin/benchmarks/aime24/prepare.py

And I think it would be easier to maintain consistency if those were all resolved in a single function, rather than separate functions.

[marta-sd]

good point. it also made me realize that in _augment_sys_path() we don't have Path.cwd() and WORKING_DIR, while we have them in component_search_roots. also I noticed that WORKING_DIR = PARENT_DIR if _is_editable_install else Path.cwd() so this looks redundant.

I will clean this up

[marta-sd]

done in 0d58030

[marta-sd]

/ok to test 7db8854

[marta-sd]

I think there's a few useful command here that are missing --search-dir
@gwarmstrong added in 7db8854

[marta-sd]

/ok to test 504b7b2