Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support associating tasks with tags, minor cleanups #21

Merged
merged 12 commits into from
Sep 10, 2024

Conversation

dmcc
Copy link
Collaborator

@dmcc dmcc commented Jul 26, 2024

tl;dr: You can now use --tags tag1,tag2 when launching tasks, fixes #20. All tags in the list will be associated with all launched tasks.

This PR also includes some overdue Ruff and pre-commit updates, addressing their changes. Got a little carried away -- ideally, these would probably be a separate PR.

dmcc added 5 commits July 26, 2024 13:58
Also addresses some minor TODOs.
Tags are working in FakeTracker, but not WandbTracker...
Likely why WandbTracker was crashing.
@dmcc dmcc added the enhancement New feature or request label Jul 26, 2024
dmcc added 2 commits July 26, 2024 14:44
Ensures that dependencies in pyproject.toml stay sorted.
src/aeromancy/wandb_tracker.py Outdated Show resolved Hide resolved
src/aeromancy/wandb_tracker.py Outdated Show resolved Hide resolved
src/aeromancy/runner.py Outdated Show resolved Hide resolved
src/aeromancy/runner.py Outdated Show resolved Hide resolved
src/aeromancy/runner.py Outdated Show resolved Hide resolved
Copy link
Contributor

@drewp drewp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I somehow can't leave a code comment on this line, but action.py:12 has an ambiguous "they", which I think should be "subclasses"

pyproject.toml Show resolved Hide resolved
@drewp
Copy link
Contributor

drewp commented Aug 9, 2024

Only now do I see I could have viewed the union of all the commits and reviewed that instead. Next time!

- `tags` should be a set, not a list. Changes throughout.
- Add `--tags` to `click_options` so it shows up in the correct group.
- Cleanups and improvements suggested by @drewpca.
Copy link
Collaborator Author

@dmcc dmcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Drew! Please take another look.

pyproject.toml Show resolved Hide resolved
src/aeromancy/runner.py Outdated Show resolved Hide resolved
src/aeromancy/runner.py Outdated Show resolved Hide resolved
src/aeromancy/runner.py Outdated Show resolved Hide resolved
src/aeromancy/wandb_tracker.py Outdated Show resolved Hide resolved
src/aeromancy/wandb_tracker.py Outdated Show resolved Hide resolved
src/aeromancy/runner.py Outdated Show resolved Hide resolved
src/aeromancy/action_runner.py Outdated Show resolved Hide resolved
We now parse CSV lists and lexicalize shell commands as Click callbacks. As a result, all recipients of Click options can avoid parsing any options from the command line and instead operate on more appropriate types.
src/aeromancy/runner.py Outdated Show resolved Hide resolved
@dmcc
Copy link
Collaborator Author

dmcc commented Aug 27, 2024

I somehow can't leave a code comment on this line, but action.py:12 has an ambiguous "they", which I think should be "subclasses"

Fixed!

Copy link
Collaborator Author

@dmcc dmcc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Drew for the many rounds of comments!

src/aeromancy/runner.py Outdated Show resolved Hide resolved
@dmcc dmcc merged commit 082d9d6 into main Sep 10, 2024
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support specifying additional tags in WandbTracker
3 participants