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

Add ability to load class instances from json config string, not just a file #881

Closed
bpkroth opened this issue Nov 14, 2024 · 0 comments · Fixed by #890
Closed

Add ability to load class instances from json config string, not just a file #881

bpkroth opened this issue Nov 14, 2024 · 0 comments · Fixed by #890
Assignees

Comments

@bpkroth
Copy link
Contributor

bpkroth commented Nov 14, 2024

This is mostly a nicety for writing interactive documentation in #869

Most user interactions with mlos_bench go through json configs, so that's really what we want to document the most.

I'd like to make those more easily browsable in the Sphinx generated docs, which means putting the json example configs into docstrings, but we want to ensure that those continue to load correctly by using doctest

Right now, ConfigPersistenceService only does load_environment via a file, but it'd be pretty easy to extend it to support a json config string as well.

@bpkroth bpkroth self-assigned this Nov 14, 2024
@bpkroth bpkroth mentioned this issue Nov 22, 2024
4 tasks
bpkroth added a commit that referenced this issue Dec 6, 2024
…for tunables (#890)

# Pull Request

## Title

Allow loading configs from JSON strings not just files and add docs for
Tunables to demonstrate and test it.

---

## Description

Adds basic support for loading a JSON config string directly.

The main purpose is to be able to show examples in docstrings that are
testable (so that they don't become stale examples).

- [x] Add basic example of usage and testing in a tunables docstring
example.
- [x] Add `pytest` unit tests for config string loading as well (we
already have file loading tests).
- Closes #881 

---

## Type of Change

- ✨ New feature
- 📝 Documentation update
- 🧪 Tests

---

## Testing

- [x] Adds additional unit tests.

---

## Additional Notes (optional)

Split out from #887 

---
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 a pull request may close this issue.

1 participant