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 use_metadata context manager (fixes #745). #746

Open
wants to merge 10 commits into
base: master
Choose a base branch
from

Conversation

tillahoffmann
Copy link

This PR adds a use_metadata context manager to specify default values for global_step and walltime. There are some usage examples in the tests, and I've updated the demo file to illustrate the use of the context manager.

I didn't quite know the right level of the codebase to add the state to. For now, I settled on the FileWriter class because all add_* seem to go through the add_event method of the FileWriter so it was easy to inject the defaults there. The SummaryWriter class has a convenience context manager method which just wraps the associated FileWriter.

I also dropped one duplicate test definition test_flush while adding the tests.

This PR currently does not support the comet logger because I wasn't quite sure about the right implementation. Comments welcome.

@tillahoffmann
Copy link
Author

It looks like there is an issue with uploading results to codecov. Do you know why that might be, @lanpa?

@lanpa
Copy link
Owner

lanpa commented Jan 15, 2025

Sorry for the late reply. For the python 3.11 error, I might have misconfigured the coverage test token. Checking it out now.

@lanpa
Copy link
Owner

lanpa commented Jan 15, 2025

Hi, I have fixed the coverage test, and the following workflow is failed. Would you take a look?

@tillahoffmann
Copy link
Author

Thanks for the commit to fix the codecov upload. Tests now pass.

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.

2 participants