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 the not-inclusive-enough isinstance checks in logging.py #5

Merged
merged 4 commits into from
Mar 21, 2024
Merged

Conversation

RolandBERTINJOHANNET
Copy link
Contributor

No description provided.

@bdvllrs
Copy link
Collaborator

bdvllrs commented Mar 21, 2024

Actually, now that I think about it, maybe a better option would be to check whether isinstance(pl_module, GlobalWorkspaceBase)?

Copy link
Collaborator

@bdvllrs bdvllrs left a comment

Choose a reason for hiding this comment

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

Except for the weird formatting, looks good!

@@ -14,9 +14,7 @@
from matplotlib.figure import Figure
from PIL import Image
from shimmer.modules.global_workspace import (
GlobalWorkspace,
GlobalWorkspaceBase,
Copy link
Collaborator

Choose a reason for hiding this comment

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

The extra "," forces ruff to format this on two lines. If you remove the "," it will probably clearner and on 1 line without extra ( )

@bdvllrs bdvllrs merged commit d29d428 into main Mar 21, 2024
2 checks passed
@bdvllrs bdvllrs deleted the fusion branch April 8, 2024 12:16
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