Skip to content
This repository has been archived by the owner on Sep 24, 2020. It is now read-only.

Windows console logging fix #170

Merged
merged 21 commits into from
Aug 31, 2020
Merged

Windows console logging fix #170

merged 21 commits into from
Aug 31, 2020

Conversation

farizrahman4u
Copy link
Contributor

No description provided.

@coveralls
Copy link

coveralls commented Aug 28, 2020

Pull Request Test Coverage Report for Build de08e7e2-1640-444e-992d-2a61013a7be0

  • 0 of 21 (0.0%) changed or added relevant lines in 3 files are covered.
  • 1 unchanged line in 1 file lost coverage.
  • Overall coverage decreased (-0.04%) to 60.442%

Changes Missing Coverage Covered Lines Changed/Added Lines %
wandb/sdk/wandb_run.py 0 1 0.0%
wandb/sdk/wandb_settings.py 0 6 0.0%
wandb/lib/redirect.py 0 14 0.0%
Files with Coverage Reduction New Missed Lines %
wandb/lib/redirect.py 1 17.83%
Totals Coverage Status
Change from base Build 15838232-c942-468e-bec9-8e58c5d846bc: -0.04%
Covered Lines: 12358
Relevant Lines: 20446

💛 - Coveralls

Copy link
Member

@raubitsj raubitsj left a comment

Choose a reason for hiding this comment

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

Based on the discussion at:
https://weightsandbiases.slack.com/archives/C0108SLEP6K/p1598636491077200
Lets:

  • Do not automatically run the registry commands
  • Try to use PYTHONLEGACYWINDOWSSTDIO
  • and if it isnt set give a message to set the registry and set the environment
  • automatically fall back to notebook console

@raubitsj
Copy link
Member

I have a PR that cleans up the console settings to make this easier. Lets get that in first:
#172

)
print(msg)
logger.info(msg)
console = self._settings.Console.NOTEBOOK
Copy link
Member

Choose a reason for hiding this comment

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

The decision of what console to use shouldnt be spread out among the code, it should in wandb_settings.py be at:
@Property
def _console(self) -> SettingsConsole:

@raubitsj raubitsj merged commit 587e630 into master Aug 31, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants