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

Logger for bioimg #474

Merged
merged 4 commits into from
Nov 15, 2023
Merged

Logger for bioimg #474

merged 4 commits into from
Nov 15, 2023

Conversation

ktsitsi
Copy link
Collaborator

@ktsitsi ktsitsi commented Oct 12, 2023

This PR:

  • Complements that PR in TileDB-Bioimaging
  • introduces a variable verbose=False that will initialise a DEBUG mode for the logger. The two implementations are not yet integrate to each other. This can easily happen by passing the verbose value of UDF argument to to_tiledb same argument.
  • The logger is being initialised by the same logger wrapper that VCF uses although we could discuss the merge of the two. I duplicate the code here cause I don't want to import a function in bioimg from vcf.ingestion.py. If agreed I could merge and move all of them under the same get_logger in utilities._common

@shortcut-integration
Copy link

This pull request has been linked to Shortcut Story #30105: Logger for Bioimg.

@ktsitsi ktsitsi requested a review from thetorpedodog October 12, 2023 16:02
@ktsitsi ktsitsi force-pushed the kt/sc-30105/logger-for-bioimg branch from 9089cc3 to 6079290 Compare November 14, 2023 12:36
@ktsitsi ktsitsi changed the title Kt/sc 30105/logger for bioimg Logger for bioimg Nov 14, 2023
Copy link
Contributor

@thetorpedodog thetorpedodog left a comment

Choose a reason for hiding this comment

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

Actually, could you separate the "single-file TileDB URIs" part of this from the logging part of this? I have a few questions about the URI change but the logging part looks OK.

@ktsitsi
Copy link
Collaborator Author

ktsitsi commented Nov 15, 2023

Actually, could you separate the "single-file TileDB URIs" part of this from the logging part of this? I have a few questions about the URI change but the logging part looks OK.

I think this is the rebased code. This part of the code was introduced, reviewed and merged yesterday here #456 . However lets discuss in case you mean a rollback of this PR.

@thetorpedodog thetorpedodog merged commit 9e4eb5d into main Nov 15, 2023
@thetorpedodog thetorpedodog deleted the kt/sc-30105/logger-for-bioimg branch November 15, 2023 15:04
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