Skip to content

Swap print() call in dataset.py with logger.info()#72

Merged
hansmohrmann merged 1 commit into
brightbandtech:mainfrom
timothyas:feature/print-to-logger
Apr 22, 2026
Merged

Swap print() call in dataset.py with logger.info()#72
hansmohrmann merged 1 commit into
brightbandtech:mainfrom
timothyas:feature/print-to-logger

Conversation

@timothyas

Copy link
Copy Markdown
Contributor

Hi folks! First off, many thanks for this amazing resource. I have created a workflow for comparing forecast models directly against observations, and I've found the nnja-ai datasets and API to be incredibly useful. Hats off to you all 👏

This PR is just a micro suggestion, so let me know what you think. Right now, every time a dataset gets loaded we get a print statement like:

Loading manifest for dataset '{self.name}'

Is there any reason this is a print statement? When I run this for example using SLURM, it results in these print statements coming at the end of the job and in a different location from the rest of the nnja_ai logging, since they're not piped in the same way as the logger. Because of that, the statements are not really useful progress markers. I think it would be nice to get these with the rest of the logs, hence the PR.

Let me know what you think! Thanks again for this awesome package!

@hansmohrmann hansmohrmann self-requested a review April 22, 2026 14:22

@hansmohrmann hansmohrmann left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Hi Tim! Apologies for the late reply. yes absolutely, this is better.

@hansmohrmann hansmohrmann reopened this Apr 22, 2026
@hansmohrmann hansmohrmann merged commit 2e6532c into brightbandtech:main Apr 22, 2026
4 checks passed
@timothyas

Copy link
Copy Markdown
Contributor Author

Awesome, thanks @hansmohrmann!

@timothyas timothyas deleted the feature/print-to-logger branch April 22, 2026 18:58
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