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

ruff fixes + type annotations #156

Merged
merged 4 commits into from
May 20, 2024
Merged

ruff fixes + type annotations #156

merged 4 commits into from
May 20, 2024

Conversation

janosh
Copy link
Collaborator

@janosh janosh commented May 19, 2024

ALL ruff rules now enabled unless specifically ignored

@BowenD-UCB needs approval given some of this is strictly speaking breaking.

  • some exception types changed from ValueError to TypeError

  • some boolean arguments that could be passed as positional before can now only be passed as keywords.

    example:

    # considered bad practice due to being unreadable, who knows what False means here
    CHGNetCalculator(some_model, some_device, False)
    
    # more readable to pass booleans as keywords, only this works now
    CHGNetCalculator(some_model, some_device, check_cuda_mem=False)

@janosh janosh added linting Cleaning up and refactoring code types Type all the things breaking Breaking change labels May 19, 2024
@janosh janosh temporarily deployed to github-pages May 19, 2024 21:25 — with GitHub Actions Inactive
'list'chgnet/model/functions.py:71: in __init__
    if hidden_dim in {None, 0}:
@janosh janosh temporarily deployed to github-pages May 19, 2024 21:28 — with GitHub Actions Inactive
@bowen-bd
Copy link
Collaborator

This is fine, thanks for the PR!

@bowen-bd bowen-bd merged commit d0632a1 into main May 20, 2024
10 checks passed
@bowen-bd bowen-bd deleted the ruff-fixes branch May 20, 2024 02:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking change linting Cleaning up and refactoring code types Type all the things
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants