chore: unify example output directories#97
Merged
Conversation
Contributor
Reviewer's GuideCentralizes example output directory handling via a new get_example_output_dir utility and refactors many example scripts to use it, so that all generated artifacts are written under examples/outputs (or an env-configured directory) instead of ad-hoc paths, while updating documentation and ignoring generated outputs from version control. Class diagram for example output directory utilityclassDiagram
class ExampleOutputsUtils {
+Path get_example_output_dir(script_path, category, base_env_var)
+Path _resolve_base_output_dir(script_path, base_env_var)
+str _infer_category(script_path)
}
class PathlibPath {
+Path resolve()
+Path expanduser()
+Path parent
+Path~list~ parents
+str name
+str stem
+Path __truediv__(other)
+None mkdir(parents, exist_ok)
+Path relative_to(parent)
+tuple parts
}
class OsEnviron {
+str get(key)
}
ExampleOutputsUtils ..> PathlibPath : uses
ExampleOutputsUtils ..> OsEnviron : reads
class ExampleScript {
+Path OUTPUT_DIR
+None main()
}
ExampleScript ..> ExampleOutputsUtils : calls get_example_output_dir
ExampleScript ..> PathlibPath : joins_paths_and_writes_files
Flow diagram for resolving example output directoriesflowchart TD
A["Example script calls\nget_example_output_dir(__file__, category)"] --> B["Normalize script_path\nusing Path.resolve"]
B --> C{"Environment variable\nCANNS_EXAMPLES_OUTPUT_DIR set?"}
C -- "Yes" --> D["base_dir = Path(env_value).expanduser"]
C -- "No" --> E{"Is script under an\nexamples/ directory?"}
E -- "Yes" --> F["base_dir = examples_dir / 'outputs'"]
E -- "No" --> G["base_dir = script_path.parent / 'outputs'"]
D --> H
F --> H
G --> H
H{"category argument is None?"} -->|Yes| I["Infer category from path:\nfirst segment under examples/\n(e.g. cann, brain_inspired)"]
H -->|No| J["Use provided category"]
I --> K
J --> K
K["Compute out_dir = base_dir / category / script_stem\n(or base_dir / script_stem if no category)"] --> L["Create out_dir with mkdir(parents=True, exist_ok=True)"]
L --> M["Return out_dir to caller\n(example uses out_dir / filename for outputs)"]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Hey - I've found 2 issues, and left some high level feedback:
- In
examples/cann/navigation_complex_environment.py, themainsignature now annotatesoutput_dir: str | PathbutPathis not imported, which will raise aNameError; add the appropriatefrom pathlib import Pathimport or adjust the annotation. - Usage of
get_example_output_dirresults in a mix ofPathandstrbeing passed assave_path(sometimes wrapped withstr(), sometimes not); consider standardizing either onPathin the called APIs or always converting tostrso the interfaces are consistent and less error-prone.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `examples/cann/navigation_complex_environment.py`, the `main` signature now annotates `output_dir: str | Path` but `Path` is not imported, which will raise a `NameError`; add the appropriate `from pathlib import Path` import or adjust the annotation.
- Usage of `get_example_output_dir` results in a mix of `Path` and `str` being passed as `save_path` (sometimes wrapped with `str()`, sometimes not); consider standardizing either on `Path` in the called APIs or always converting to `str` so the interfaces are consistent and less error-prone.
## Individual Comments
### Comment 1
<location> `src/canns/utils/example_outputs.py:22-25` </location>
<code_context>
+ return out_dir
+
+
+def _resolve_base_output_dir(script_path: Path, base_env_var: str) -> Path:
+ base_env = os.environ.get(base_env_var)
+ if base_env:
+ return Path(base_env).expanduser()
</code_context>
<issue_to_address>
**suggestion (bug_risk):** Consider normalizing the environment-provided base directory with `.resolve()` for more predictable behavior.
When `base_env_var` is set, `Path(base_env).expanduser()` may still be relative to the current working directory, so behavior can change depending on where the script is launched. Using `.expanduser().resolve()` makes the path deterministic and consistent with how `script_path` is handled in `get_example_output_dir`. For example:
```python
base_env = os.environ.get(base_env_var)
if base_env:
return Path(base_env).expanduser().resolve()
```
```suggestion
def _resolve_base_output_dir(script_path: Path, base_env_var: str) -> Path:
base_env = os.environ.get(base_env_var)
if base_env:
return Path(base_env).expanduser().resolve()
```
</issue_to_address>
### Comment 2
<location> `examples/README.md:13` </location>
<code_context>
+ (Hopfield, Oja, BCM, STDP, etc.)
+- `cann/`: continuous attractor neural network (CANN) models and navigation
+ demos
+- `cell_classification/`: grid / cell classification examples
+- `experimental_data_analysis/`: ASA pipeline utilities (TDA, decoding,
+ CohoMap/CohoSpace, path compare, firing-rate maps)
</code_context>
<issue_to_address>
**nitpick (typo):** Consider clarifying the phrase "grid / cell classification examples"
This phrasing is ambiguous; if you mean classification of grid cells, consider something like “grid-cell classification examples” or “grid cell classification examples” for clarity.
```suggestion
- `cell_classification/`: grid-cell classification examples
```
</issue_to_address>Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Testing
Summary by Sourcery
Unify example script output handling around a shared helper that writes artifacts into a consistent examples/outputs directory hierarchy.
Enhancements:
Documentation:
Chores: