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

[Refactoring] resolved some TODOs items from the source code #1496

Merged
merged 13 commits into from May 26, 2022

Conversation

yromanyshyn
Copy link
Contributor

resolved some TODOs from the source code

list of changes:

  • added context manager for the wandb run creation
  • added comments for the collected check results indexes in the SuiteResult constructor
  • added deprecation warning for the with_display parameter (CheckFailure.to_json, SuiteResult.to_json)

@yromanyshyn yromanyshyn added the refactoring Making significant changes to structure of code label May 23, 2022
@yromanyshyn yromanyshyn self-assigned this May 23, 2022
@yromanyshyn yromanyshyn requested review from a team as code owners May 23, 2022 10:48
@ItayGabbay
Copy link
Member

@JKL98ISR can you review?

@ItayGabbay ItayGabbay requested a review from JKL98ISR May 23, 2022 13:04
deepchecks/core/suite.py Outdated Show resolved Hide resolved
@yromanyshyn
Copy link
Contributor Author

@JKL98ISR

note regarding to_wandb method

I marked dedicated_run parameter as deprecated because there is no sense in having it.
It looks like wandb.run is a singleton and you cannot have two or more instances of it simultaneously.

example

import wandb

r1 = wandb.init(project='test1')
r2 = wandb.init(project='test2')

print(wandb.run is r2)  # True
r2.log({'accuraccy': 0.8})
r2.finish()

print(wandb.run is None)  # True
r1.log({'accuraccy': 0.8})  # << will raise "Exception: The wandb backend process has shutdown"    

so, now to_wandb method works in the next way:
if wandb.run is not None, use it otherwise initialize new instance and close it at the end

@yromanyshyn yromanyshyn enabled auto-merge (squash) May 26, 2022 11:38
@yromanyshyn yromanyshyn requested a review from matanper May 26, 2022 14:33
@noamzbr noamzbr disabled auto-merge May 26, 2022 14:39
@noamzbr noamzbr merged commit ac6412d into main May 26, 2022
@delete-merged-branch delete-merged-branch bot deleted the wandb-context-manager branch May 26, 2022 14:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
refactoring Making significant changes to structure of code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants