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

changed config in two files #131

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

ACSE-vg822
Copy link

Helps: #126
What has changed?

  • added dacite to requirements.txt
  • added type checking using dacite for configuration in two places

@jacobbieker these are the two instances I found that needed this change, please let me know where else I need to do it.

@jacobbieker
Copy link
Member

Hi, this is a great start! We also want to have the dacite configurations used for the general torch modules, such as in

class GraphWeatherForecaster(torch.nn.Module, PyTorchModelHubMixin):
and the other PyTorch modules that have configurations.

@jacobbieker jacobbieker self-requested a review January 27, 2025 09:15
@ACSE-vg822
Copy link
Author

Alright, thank you! will find and do the rest :)
However, I don't understand why ruff is failing(for files I didn't commit). Am I missing something?

@ACSE-vg822 ACSE-vg822 marked this pull request as draft January 28, 2025 13:11
@jacobbieker
Copy link
Member

Alright, thank you! will find and do the rest :) However, I don't understand why ruff is failing(for files I didn't commit). Am I missing something?

Yeah, the pre-commit is broken right now on this repo. I wouldn't worry about it. I'll fix it at some point, for now its fine if its failing.

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