Skip to content

Branch ci lint typecheck fix - #540

Merged
lm2612 merged 18 commits into
mainfrom
branch_ci_lint_typecheck_fix
Jul 31, 2026
Merged

Branch ci lint typecheck fix#540
lm2612 merged 18 commits into
mainfrom
branch_ci_lint_typecheck_fix

Conversation

@lm2612

@lm2612 lm2612 commented Jul 31, 2026

Copy link
Copy Markdown
Contributor

Pull Request

Description

Following .github/workflows changes and ruff update to v0.16, some linting issues were flagged which leads to CI failing. I have implementing some fixes to the code and added some ignore error codes to the pyproject.toml for the following:

  • "TRY002", # Create your own exception
  • "LOG015", # warning() call on root logger
  • "B008", # Do not perform function call typer.Argument

Fixes #537

How Has This Been Tested?

The updates should not change any of the code. pytests still run. I've also tested that I can train pvnet

  • Yes

Checklist:

  • My code follows OCF's coding style guidelines
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • I have checked my code and corrected any misspellings

@lm2612
lm2612 marked this pull request as ready for review July 31, 2026 10:54
Comment thread pyproject.toml
[tool.ruff.lint]
extend-select = ["E", "D", "I"]
ignore = ["D200","D202","D210","D212","D415","D105"]
ignore = ["D200","D202","D210","D212","D415","D105",

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Avoids the following errors which I was unsure how to deal with:

TRY002 Create your own exception
   --> pvnet/models/base_model.py:166:23
    |
164 |           except Exception as e:
165 |               if attempt == max_retries:
166 |                   raise Exception(
    |  _______________________^
167 | |                     f"Failed to download {filename} from {repo_id} after {max_retries} attempts."
168 | |                 ) from e


LOG015 `warning()` call on root logger
   --> pvnet/models/base_model.py:169:13
    |
167 |                       f"Failed to download {filename} from {repo_id} after {max_retries} attempts."
168 |                   ) from e
169 | /             logging.warning(
170 | |                 (
171 | |                     f"Attempt {attempt}/{max_retries} failed to download {filename} "
172 | |                     f"from {repo_id}. Retrying in {wait_time} seconds..."
173 | |                 )
174 | |             )

B008 Do not perform function call `typer.Argument` in argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable
  --> scripts/checkpoint_to_huggingface.py:33:39
   |
31 | @app.command()
32 | def push_to_huggingface(
33 |     checkpoint_dir_paths: list[str] = typer.Argument(...,),
   |                                       ^^^^^^^^^^^^^^^^^^^^
34 |     huggingface_repo: str = typer.Option(..., "--huggingface-repo"),
35 |     wandb_repo: str = typer.Option(..., "--wandb-repo"),

MODEL_CONFIG_NAME,
PYTORCH_WEIGHTS_NAME,
)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Should add logger = logging.getLogger(__name__)

Comment thread pyproject.toml Outdated
ignore = ["D200","D202","D210","D212","D415","D105"]
ignore = ["D200","D202","D210","D212","D415","D105",
"TRY002", # Create your own exception
"LOG015", # `warning()` call on root logger

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

try to remove this, using logger = logging.getLogger(__name__)

Comment thread pyproject.toml
"TRY002", # Create your own exception
"LOG015", # `warning()` call on root logger
"B008", # Do not perform function call typer.Argument
]

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

happy to ignore TRY002 and B008

@peterdudfield

Copy link
Copy Markdown
Contributor

Approved, but good to change logger warning

@lm2612
lm2612 merged commit 0e93f4b into main Jul 31, 2026
4 checks passed
@lm2612
lm2612 deleted the branch_ci_lint_typecheck_fix branch July 31, 2026 11:55
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.

CI lint checks failing

2 participants