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

feat: add pre-commit hooks #73

Closed
wants to merge 9 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .pre-commit-hooks.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
- id: dargs-check
name: dargs-check
description: "Run 'dargs check' to check JSON files."
entry: dargs check
language: python
types_or: [json]
minimum_pre_commit_version: "2.9.2"
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ Please refer to test files for detailed usage.
## Additional features

- [PEP 484](https://peps.python.org/pep-0484/) type annotations
- Check JSON files using [pre-commit](https://github.com/pre-commit/pre-commit)
- Native integration with [Sphinx](https://github.com/sphinx-doc/sphinx), [DP-GUI](https://github.com/deepmodeling/dpgui), and [Jupyter Notebook](https://jupyter.org/)
- JSON encoder for `Argument` and `Variant` classes
- Generate [JSON schema](https://json-schema.org/) from an `Argument`, which can be further integrated with JSON editors such as [Visual Studio Code](https://code.visualstudio.com/)
2 changes: 1 addition & 1 deletion dargs/cli.py
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@
dict
normalized data
"""
module_name, attr_name = func.rsplit(".", 1)
module_name, attr_name = func.strip().rsplit(".", 1)

Check warning on line 93 in dargs/cli.py

View check run for this annotation

Codecov / codecov/patch

dargs/cli.py#L93

Added line #L93 was not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

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

LGTM! Consider adding tests for the new functionality.

The addition of strip() ensures that any leading or trailing whitespace in the func string is removed before splitting it to derive module_name and attr_name. This is a useful change to prevent potential errors due to extra whitespace.

However, the new line is not covered by tests.

Do you want me to generate the unit testing code or open a GitHub issue to track this task?

Tools
GitHub Check: codecov/patch

[warning] 93-93: dargs/cli.py#L93
Added line #L93 was not covered by tests

try:
mod = __import__(module_name, globals(), locals(), [attr_name])
except ImportError as e:
Expand Down
1 change: 1 addition & 0 deletions docs/index.rst
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ Welcome to dargs's documentation!

intro
cli
pre-commit
sphinx
dpgui
nb
Expand Down
16 changes: 16 additions & 0 deletions docs/pre-commit.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# Pre-commit hooks

To check JSON files (available as of dargs v0.4.8) via [pre-commit](https://github.com/pre-commit/pre-commit), add the following to your `.pre-commit-config.yaml`:

```yaml
- repo: https://github.com/deepmodeling/dargs
rev: v0.4.8
hooks:
- id: dargs-check
# Specify the function to return arguments
args: ["-f dargs._test.test_arguments"]
# Pattern of JSON files to check
files: "tests/test_arguments.json"
# Add additional Python dependencies for arguments
additional_dependencies: []
```