JOSS manuscript #48
JOSS manuscript #48
Conversation
for more information, see https://pre-commit.ci
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #48 +/- ##
=======================================
Coverage 97.33% 97.33%
=======================================
Files 25 25
Lines 1723 1723
=======================================
Hits 1677 1677
Misses 46 46 ☔ View full report in Codecov by Sentry. |
WalkthroughThis PR updates tooling configs, adds a paper manuscript and bibliography, introduces a Docker-based paper build script, and expands ignore rules. No code or public API changes were made. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
✨ Finishing Touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
.pre-commit-config.yaml (1)
36-40: Optional: Re-enable mdformat only for paper/ with frontmatter supportIf formatting drift becomes a concern, you can scope mdformat to paper/*.md and preserve YAML front matter.
Apply this diff to re-enable with safe scope:
- # - repo: https://github.com/executablebooks/mdformat - # rev: 0.7.22 - # hooks: - # - id: mdformat - # args: ["--wrap=80"] + - repo: https://github.com/executablebooks/mdformat + rev: 0.7.22 + hooks: + - id: mdformat + additional_dependencies: ["mdformat-gfm", "mdformat-frontmatter"] + files: ^paper/.*\.md$ + args: ["--wrap=80"]paper/paper.md (3)
83-83: Typo: double periodSingle period is sufficient.
-This bidirectional conversion capability allows the use of RDKit's chemical analysis tools together with ASE for MLIP-based simulations.. +This bidirectional conversion capability allows the use of RDKit's chemical analysis tools together with ASE for MLIP-based simulations.
89-89: Unify code-fence language identifierUse “python” consistently for syntax highlighting.
-```py +```python
39-41: Style: tighten “not only … but also …” phrasingOptional clarity edit.
-This simplification not only accelerates research but also supports the setup of more complex and chemically diverse simulation scenarios. +This simplification accelerates research and supports the setup of more complex, chemically diverse simulation scenarios.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
paper/zndraw_rdkit.svgis excluded by!**/*.svg
📒 Files selected for processing (6)
.pre-commit-config.yaml(1 hunks)paper/.gitignore(1 hunks)paper/bibliography.bib(1 hunks)paper/build.sh(1 hunks)paper/paper.md(1 hunks)pyproject.toml(1 hunks)
🧰 Additional context used
🪛 LanguageTool
paper/paper.md
[style] ~40-~40: Opting for a less wordy alternative here may improve the clarity of your writing.
Context: ...f simulation cells. This simplification not only accelerates research but also supports the setup of more complex and chemicall...
(NOT_ONLY_ALSO)
🪛 Shellcheck (0.10.0)
paper/build.sh
[warning] 3-3: Quote this to prevent word splitting.
(SC2046)
[warning] 3-3: Quote this to prevent word splitting.
(SC2046)
🔇 Additional comments (4)
paper/.gitignore (1)
1-5: LGTM – appropriate ignores for paper artifactsThe rules cover common build outputs (JATS, PDFs, notebooks, PNGs). No issues.
pyproject.toml (1)
51-52: LGTM – codespell config tuned to reduce false positivesAdding “basf” and skipping SVGs and the BibTeX file is pragmatic for this PR.
paper/bibliography.bib (1)
1-212: Bibliography looks consistent and completeReferences cover the cited tooling and concepts; field usage and keys are consistent. No changes required.
paper/paper.md (1)
49-49: Figure asset confirmed present
The filepaper/zndraw_rdkit.svgwas found in the repository underpaper/, so no further action is required.
| #!/bin/bash | ||
|
|
||
| docker run --rm --volume $PWD:/data --user $(id -u):$(id -g) --platform=linux/amd64 --env JOURNAL=joss openjournals/inara |
There was a problem hiding this comment.
Harden the build script (quote expansions, fail-fast, ensure correct CWD, check docker)
Quoting avoids word-splitting; setting safety flags and cd’ing to the script directory makes the build reliable from any location.
Apply:
-#!/bin/bash
-
-docker run --rm --volume $PWD:/data --user $(id -u):$(id -g) --platform=linux/amd64 --env JOURNAL=joss openjournals/inara
+#!/bin/bash
+set -Eeuo pipefail
+
+# Run from the paper directory so /data matches the manuscript location.
+cd "$(dirname "$0")"
+
+# Ensure docker is available.
+if ! command -v docker >/dev/null 2>&1; then
+ echo "docker is required but not found in PATH." >&2
+ exit 127
+fi
+
+# Build the JOSS paper using the Inara container.
+docker run --rm \
+ --pull=always \
+ --volume "$PWD":/data \
+ --user "$(id -u)":"$(id -g)" \
+ --platform=linux/amd64 \
+ --env JOURNAL=joss \
+ openjournals/inara📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| #!/bin/bash | |
| docker run --rm --volume $PWD:/data --user $(id -u):$(id -g) --platform=linux/amd64 --env JOURNAL=joss openjournals/inara | |
| #!/bin/bash | |
| set -Eeuo pipefail | |
| # Run from the paper directory so /data matches the manuscript location. | |
| cd "$(dirname "$0")" | |
| # Ensure docker is available. | |
| if ! command -v docker >/dev/null 2>&1; then | |
| echo "docker is required but not found in PATH." >&2 | |
| exit 127 | |
| fi | |
| # Build the JOSS paper using the Inara container. | |
| docker run --rm \ | |
| --pull=always \ | |
| --volume "$PWD":/data \ | |
| --user "$(id -u)":"$(id -g)" \ | |
| --platform=linux/amd64 \ | |
| --env JOURNAL=joss \ | |
| openjournals/inara |
🧰 Tools
🪛 Shellcheck (0.10.0)
[warning] 3-3: Quote this to prevent word splitting.
(SC2046)
[warning] 3-3: Quote this to prevent word splitting.
(SC2046)
🤖 Prompt for AI Agents
In paper/build.sh around lines 1 to 3, the script lacks safety flags, unquoted
expansions, no cwd enforcement, and no check for docker availability; update the
script to: enable fail-fast and strict mode (set -euo pipefail), change to the
script directory (cd "$(dirname "$0")"), verify docker is installed and
executable before proceeding (exit with a clear error if not), and quote all
expansions (e.g. use --volume "$PWD":/data and --user "$(id -u):$(id -g)") so
paths and values with spaces are handled correctly.
Summary by CodeRabbit
Documentation
Chores