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

Fix CometML tests #585

Merged
merged 2 commits into from
Dec 7, 2019
Merged

Conversation

neggert
Copy link
Contributor

@neggert neggert commented Dec 4, 2019

In master, the CometML tests print a bunch of nasty warning messages because the comet_ml library uses atexit.register to print a summary. This conflicts with pytest's redirecting of stdout and stderr. More info here: pytest-dev/pytest#5577. Fixed it by monkeypatching atexit.register to do nothing.

While I was in there, I noticed that the version property behaves differently than for the other loggers, so I fixed it.

@neggert
Copy link
Contributor Author

neggert commented Dec 5, 2019

Not sure why Travis failed. Looks unrelated to the PR. Do we have a way to re-run it?

@williamFalcon williamFalcon merged commit 0489e31 into Lightning-AI:master Dec 7, 2019
@Borda
Copy link
Member

Borda commented Dec 7, 2019

Not sure why Travis failed. Looks unrelated to the PR. Do we have a way to re-run it?

There is probably some cache problem #590

@neggert neggert deleted the comet_ml_fixes branch December 7, 2019 15:41
Lothiraldan added a commit to comet-ml/pytorch-lightning that referenced this pull request Jul 8, 2020
The rest_api_key was only used for creating a Comet API object previously used
to compute the Logger version based on the number but this was cleaned by Lightning-AI#585
and the code was likely left there.
Lothiraldan added a commit to comet-ml/pytorch-lightning that referenced this pull request Jul 8, 2020
The rest_api_key was only used for creating a Comet API object previously used
to compute the Logger version based on the number but this was cleaned by Lightning-AI#585
and the code was likely left there.
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.

3 participants