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. |
WalkthroughAdds a JOSS-style paper and bibliography, a Docker-based paper build script, updates project/tooling configs and ignore rules, and disables the mdformat pre-commit hook. No code or public API changes. Changes
Sequence Diagram(s)sequenceDiagram
participant Dev as Developer
participant Script as paper/build.sh
participant Docker as Docker Engine
participant Image as openjournals/inara
Dev->>Script: run ./paper/build.sh
Script->>Docker: docker run --rm ... --volume $PWD:/data --env JOURNAL=joss openjournals/inara
Docker->>Image: start container (runs build inside /data)
Image-->>Docker: build outputs (paper, jats, logs)
Docker-->>Script: container exits (files written to host /data)
Script-->>Dev: exit status / artifacts available
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Points to check during review:
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
![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.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
paper/paper.md (1)
26-26: Optional grammar and hyphenation refinements.Static analysis flagged three minor style opportunities:
- Line 26: "high throughput simulations" → "high-throughput simulations" (compound adjective requires hyphenation)
- Line 35: "machine-learning driven bond order" → "machine-learning-driven bond order" (consistent hyphenation in compound adjectives)
- Line 44: "This simplification not only accelerates research but also supports" could be simplified to "This simplification accelerates research and supports" (reduces verbosity without sacrificing clarity)
These are stylistic improvements and not required for acceptance; apply at your discretion.
Also applies to: 35-35, 44-44
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.pre-commit-config.yaml(1 hunks)paper/bibliography.bib(1 hunks)paper/paper.md(1 hunks)pyproject.toml(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- paper/bibliography.bib
- .pre-commit-config.yaml
🧰 Additional context used
🪛 LanguageTool
paper/paper.md
[grammar] ~26-~26: Use a hyphen to join words.
Context: ... run on much larger systems, making high throughput simulations of more complex s...
(QB_NEW_EN_HYPHEN)
[grammar] ~35-~35: Use a hyphen to join words.
Context: ...ons or integrating with machine-learning driven bond order predictions. # State...
(QB_NEW_EN_HYPHEN)
[style] ~44-~44: 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)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (7)
- GitHub Check: pytest (3.10, windows-latest)
- GitHub Check: Build wheels on windows-11-arm
- GitHub Check: Build wheels on windows-latest
- GitHub Check: Build wheels on ubuntu-latest
- GitHub Check: Build wheels on ubuntu-24.04-arm
- GitHub Check: pytest (3.12, windows-latest)
- GitHub Check: pytest (3.13, macos-latest)
🔇 Additional comments (2)
pyproject.toml (1)
67-68: Codespell configuration appropriately updated for paper tooling.The additions to
ignore-words-listandskippatterns are well-justified: "basf" prevents false positives from the BASF organization (referenced in Related software), while the skip patterns accommodate SVG figures, the new bibliography file, and notebook artifacts. This prevents Codespell from flagging abbreviations and specialized terms common in academic bibliographies and illustrations.paper/paper.md (1)
1-128: Paper structure and technical content are well-executed.The JOSS paper comprehensively documents molify's problem space, features, and role within the materials science/MLIP community. The metadata, sections (Summary, Statement of Need, Features, Acknowledgements, Related Software), code examples, and bibliography references follow standard conventions. The comparison with OpenBabel (lines 126–128) effectively distinguishes molify's Python-native, in-memory workflow focus from format-conversion-oriented tools.
2 participants
Summary by CodeRabbit
Documentation
Chores