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

Fix: ignore UP031 #762

Merged
merged 2 commits into from
Dec 4, 2024
Merged

Fix: ignore UP031 #762

merged 2 commits into from
Dec 4, 2024

Conversation

wanghan-iapcm
Copy link
Contributor

@wanghan-iapcm wanghan-iapcm commented Dec 4, 2024

Summary by CodeRabbit

  • New Features
    • Introduced the SQMMinimizer class for enhanced data minimization.
  • Improvements
    • Enhanced error handling in the sanitize_mol function.
    • Improved comments for clarity and linting purposes across various modules.
  • Bug Fixes
    • Updated assertion messages for better clarity in multiple functions.
  • Documentation
    • Added comments to suppress linting warnings in several functions, improving code readability.

Copy link

codspeed-hq bot commented Dec 4, 2024

CodSpeed Performance Report

Merging #762 will not alter performance

Comparing wanghan-iapcm:fix-up031 (e6a66c6) with devel (467ffbd)

Summary

✅ 2 untouched benchmarks

Copy link

codecov bot commented Dec 4, 2024

Codecov Report

Attention: Patch coverage is 45.16129% with 17 lines in your changes missing coverage. Please review.

Project coverage is 85.15%. Comparing base (467ffbd) to head (e6a66c6).
Report is 1 commits behind head on devel.

Files with missing lines Patch % Lines
dpdata/system.py 0.00% 4 Missing ⚠️
dpdata/plugins/amber.py 0.00% 2 Missing ⚠️
dpdata/plugins/gaussian.py 0.00% 2 Missing ⚠️
dpdata/rdkit/sanitize.py 0.00% 2 Missing ⚠️
dpdata/vasp/xml.py 0.00% 2 Missing ⚠️
dpdata/abacus/relax.py 0.00% 1 Missing ⚠️
dpdata/deepmd/hdf5.py 50.00% 1 Missing ⚠️
dpdata/deepmd/raw.py 0.00% 1 Missing ⚠️
dpdata/gaussian/gjf.py 50.00% 1 Missing ⚠️
dpdata/lammps/lmp.py 66.66% 1 Missing ⚠️
Additional details and impacted files
@@           Coverage Diff           @@
##            devel     #762   +/-   ##
=======================================
  Coverage   85.15%   85.15%           
=======================================
  Files          81       81           
  Lines        7526     7526           
=======================================
  Hits         6409     6409           
  Misses       1117     1117           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

coderabbitai bot commented Dec 4, 2024

Warning

Rate limit exceeded

@wanghan-iapcm has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 17 minutes and 1 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

📥 Commits

Reviewing files that changed from the base of the PR and between f0f8c27 and e6a66c6.

📝 Walkthrough
📝 Walkthrough

Walkthrough

This pull request primarily updates the .pre-commit-config.yaml file to change the version of the ruff-pre-commit repository from v0.7.4 to v0.8.1. Additionally, it includes various modifications across multiple Python files, focusing on adding comments to suppress specific linting warnings without altering the core functionality or logic of the code. The changes span several modules, including updates to error messages, string formatting, and the introduction of a new class and method in the dpdata/plugins/amber.py file.

Changes

File Path Change Summary
.pre-commit-config.yaml Updated ruff-pre-commit version from v0.7.4 to v0.8.1.
dpdata/abacus/md.py Updated assertion messages with comments for linter suppression.
dpdata/abacus/relax.py Added comments for linter suppression in assertions.
dpdata/data_type.py Modified error message formatting in check method for linter suppression.
dpdata/deepmd/comp.py Added comments for linter suppression in dump function.
dpdata/deepmd/hdf5.py Added comments for linter suppression in to_system_data and dump functions.
dpdata/deepmd/raw.py Added comment for linter suppression in load_type function.
dpdata/gaussian/gjf.py Updated string formatting in make_gaussian_input function with comments for linter suppression.
dpdata/lammps/dump.py Added comment for linter suppression in system_data function.
dpdata/lammps/lmp.py Added comments for linter suppression in from_system_data and system_data functions.
dpdata/plugins/amber.py Introduced SQMMinimizer class and updated SQMDriver class with comments for linter suppression.
dpdata/plugins/gaussian.py Added comments for linter suppression in GaussianDriver class.
dpdata/pwmat/atomconfig.py Added comments for linter suppression in from_system_data function.
dpdata/qe/traj.py Added comments for linter suppression in load_celldm and load_energy functions.
dpdata/rdkit/sanitize.py Enhanced sanitization process with super_sanitize_mol method and updated error handling.
dpdata/system.py Updated LabeledSystem class to inherit post_funcs and added comments for linter suppression.
dpdata/vasp/poscar.py Added comments for linter suppression in from_system_data function.
dpdata/vasp/xml.py Added comments for linter suppression in formulate_config function.

Possibly related PRs


Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 1

🧹 Outside diff range and nitpick comments (3)
dpdata/plugins/amber.py (1)

139-140: Consider using f-strings for better readability

While the current %-formatting with noqa: UP031 works, f-strings would be more Pythonic and eliminate the need for the linting suppressions:

-                inp_fn = os.path.join(d, "%d.in" % ii)  # noqa: UP031
-                out_fn = os.path.join(d, "%d.out" % ii)  # noqa: UP031
+                inp_fn = os.path.join(d, f"{ii}.in")
+                out_fn = os.path.join(d, f"{ii}.out")

However, since this PR's focus is on fixing UP031 warnings, the current approach with noqa comments is acceptable.

dpdata/abacus/relax.py (1)

50-50: Consider using f-strings for error messages

While the noqa comment addresses the UP031 warning, consider using f-strings for better readability:

-        "ERROR: detected atom number in log file is %d" % natoms  # noqa: UP031
+        f"ERROR: detected atom number in log file is {natoms}"
dpdata/rdkit/sanitize.py (1)

370-370: Consider modernizing string formatting throughout the file.

While the current changes to suppress warnings are acceptable, consider modernizing the codebase to use f-strings consistently throughout the file. This would improve readability and follow modern Python practices.

Example refactor:

- mol.SetProp("edit", "%d_%d" % (i, j))  # noqa: UP031
+ mol.SetProp("edit", f"{i}_{j}")

- mol.SetProp("edit", edited + ",%d_%d" % (i, j))  # noqa: UP031
+ mol.SetProp("edit", f"{edited},{i}_{j}")

Also applies to: 373-373

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 467ffbd and f0f8c27.

📒 Files selected for processing (18)
  • .pre-commit-config.yaml (1 hunks)
  • dpdata/abacus/md.py (4 hunks)
  • dpdata/abacus/relax.py (2 hunks)
  • dpdata/data_type.py (1 hunks)
  • dpdata/deepmd/comp.py (2 hunks)
  • dpdata/deepmd/hdf5.py (2 hunks)
  • dpdata/deepmd/raw.py (1 hunks)
  • dpdata/gaussian/gjf.py (2 hunks)
  • dpdata/lammps/dump.py (1 hunks)
  • dpdata/lammps/lmp.py (6 hunks)
  • dpdata/plugins/amber.py (1 hunks)
  • dpdata/plugins/gaussian.py (1 hunks)
  • dpdata/pwmat/atomconfig.py (2 hunks)
  • dpdata/qe/traj.py (2 hunks)
  • dpdata/rdkit/sanitize.py (1 hunks)
  • dpdata/system.py (3 hunks)
  • dpdata/vasp/poscar.py (2 hunks)
  • dpdata/vasp/xml.py (2 hunks)
✅ Files skipped from review due to trivial changes (14)
  • dpdata/abacus/md.py
  • dpdata/data_type.py
  • dpdata/pwmat/atomconfig.py
  • dpdata/deepmd/comp.py
  • dpdata/vasp/xml.py
  • dpdata/vasp/poscar.py
  • dpdata/gaussian/gjf.py
  • dpdata/qe/traj.py
  • dpdata/plugins/gaussian.py
  • dpdata/deepmd/raw.py
  • dpdata/deepmd/hdf5.py
  • dpdata/lammps/dump.py
  • dpdata/lammps/lmp.py
  • dpdata/system.py
🧰 Additional context used
🪛 Ruff (0.8.0)
dpdata/abacus/relax.py

79-79: Do not assert False (python -O removes these calls), raise AssertionError()

Replace assert False

(B011)

🔇 Additional comments (2)
.pre-commit-config.yaml (1)

24-24: Verify compatibility with ruff v0.8.1

The update from v0.7.4 to v0.8.1 is significant. Let's verify there are no breaking changes that could affect the codebase.

✅ Verification successful

The update to ruff v0.8.1 appears safe to proceed

The codebase shows extensive Python files that would be affected by ruff linting. The pre-commit configuration is properly structured with ruff configured to run in fix mode, which will automatically correct any fixable issues. Since ruff follows semantic versioning, the update from v0.7.4 to v0.8.1 should maintain backward compatibility for existing rules.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for breaking changes between ruff versions

# Get the changelog for the version update
curl -s https://raw.githubusercontent.com/astral-sh/ruff/main/CHANGELOG.md | grep -A 10 "## \[0.8.1\]"

# Check if there are any remaining UP031 violations after the update
ruff check --select UP031 .

Length of output: 229


Script:

#!/bin/bash
# Let's check the codebase for any Python files and potential UP031 violations
# using ast-grep since ruff is not available in the sandbox

# First, let's find all Python files
fd -e py

# Check for any instances of 'print' function calls without parentheses
# which is what UP031 typically flags
ast-grep --pattern 'print $_'

# Let's also check the pre-commit config details
cat .pre-commit-config.yaml

Length of output: 6339

dpdata/rdkit/sanitize.py (1)

370-370: LGTM! Suppressing UP031 warnings is appropriate here.

The addition of # noqa: UP031 comments to suppress the string formatting warnings is acceptable as it's part of a targeted effort to address specific linting issues.

Also applies to: 373-373

dpdata/abacus/relax.py Show resolved Hide resolved
@njzjz njzjz merged commit 5423efe into deepmodeling:devel Dec 4, 2024
11 of 12 checks passed
@wanghan-iapcm wanghan-iapcm deleted the fix-up031 branch December 5, 2024 00:47
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