-
Notifications
You must be signed in to change notification settings - Fork 137
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
docs: fix directives #715
docs: fix directives #715
Conversation
The previous syntax is for RST files.
Warning Rate limit exceeded@njzjz has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 24 minutes and 35 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the 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. WalkthroughWalkthroughThe recent changes involve updates to the documentation across multiple files, primarily focusing on the formatting of class references from a colon-based syntax to a curly brace format. Additionally, a new Changes
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? TipsChatThere are 3 ways to chat with CodeRabbit:
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)
Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## devel #715 +/- ##
=======================================
Coverage 84.50% 84.50%
=======================================
Files 81 81
Lines 7152 7152
=======================================
Hits 6044 6044
Misses 1108 1108 ☔ View full report in Codecov by Sentry. |
CodSpeed Performance ReportMerging #715 will not alter performanceComparing Summary
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (6)
docs/systems/multi.md (2)
5-6
: Approve syntax changes and suggest minor formatting improvementThe syntax for referencing methods and classes has been correctly updated. However, there are some extra spaces that could be removed for better readability.
Consider applying this diff to improve formatting:
-Use {meth}`dpdata.MultiSystems.from_dir` to read from a directory, {class}`dpdata.MultiSystems` will walk in the directory -Recursively and find all file with specific file_name. Supports all the file formats that {class}`dpdata.LabeledSystem` supports. +Use {meth}`dpdata.MultiSystems.from_dir` to read from a directory, {class}`dpdata.MultiSystems` will walk in the directory +recursively and find all files with specific file_name. Supports all the file formats that {class}`dpdata.LabeledSystem` supports.
10-10
: Suggest minor grammatical improvementThere's a minor grammatical issue in this line. Consider adding an article for better readability.
Apply this change to improve grammar:
-For example, for `quip/gap xyz` files, single .xyz file may contain many different configurations with different atom numbers and atom type. +For example, for `quip/gap xyz` files, a single .xyz file may contain many different configurations with different atom numbers and atom types.Tools
LanguageTool
[uncategorized] ~10-~10: Possible missing article found.
Context: ... For example, forquip/gap xyz
files, single .xyz file may contain many different co...(AI_HYDRA_LEO_MISSING_A)
docs/systems/bond_order_system.md (4)
3-3
: Improve clarity and correct grammar in the introduction.The introduction provides a good overview of the
BondOrderSystem
class, but there are a few areas for improvement:
- Replace "information of chemical bonds" with "information on chemical bonds" for better grammar.
- Consider adding a brief explanation of why this new class is useful or what problem it solves.
- The sentence about format conversion is important but could be more prominent.
Here's a suggested revision:
-A new class {class}`BondOrderSystem` which inherits from class {class}`System` is introduced in dpdata. This new class contains information of chemical bonds and formal charges (stored in `BondOrderSystem.data['bonds']`, `BondOrderSystem.data['formal_charges']`). Now BondOrderSystem can only read from .mol/.sdf formats, because of its dependency on rdkit (which means rdkit must be installed if you want to use this function). Other formats, such as pdb, must be converted to .mol/.sdf format (maybe with software like open babel). +A new class {class}`BondOrderSystem`, which inherits from class {class}`System`, has been introduced in dpdata. This class enhances molecular representation by storing information on chemical bonds and formal charges (in `BondOrderSystem.data['bonds']` and `BondOrderSystem.data['formal_charges']` respectively). + +Currently, BondOrderSystem can only read from .mol/.sdf formats due to its dependency on rdkit. To use this functionality, rdkit must be installed. + +**Important:** Other formats, such as PDB, must be converted to .mol or .sdf format before use. This conversion can be done using external tools like Open Babel.Tools
LanguageTool
[grammar] ~3-~3: The correct preposition here is “on” or “about”.
Context: ...ta. This new class contains information of chemical bonds and formal charges (stor...(INFORMATION_OF)
15-15
: Approve changes and suggest minor improvement.The addition of information about initializing from an rdkit Mol object is valuable. The code examples effectively demonstrate the usage of
BondOrderSystem
.Consider adding a brief comment in the code example to explain what each step does, especially for the rdkit operations. This would make the example more educational for users who might be less familiar with rdkit. For example:
from rdkit import Chem from rdkit.Chem import AllChem import dpdata mol = Chem.MolFromSmiles("CC") # Create a molecule from SMILES mol = Chem.AddHs(mol) # Add hydrogen atoms AllChem.EmbedMultipleConfs(mol, 10) # Generate 3D conformers system = dpdata.BondOrderSystem(rdkit_mol=mol) # Initialize BondOrderSystem from rdkit Mol
28-28
: Improve grammar and clarity in the Bond Order Assignment section.The explanation of sanitization levels is informative, but there are some minor issues to address:
- Change "3 level of sanitization" to "3 levels of sanitization".
- Remove the colon after "by" for better grammatical structure.
- Consider reorganizing the information for better readability.
Here's a suggested revision:
-The {class}`BondOrderSystem` implements a more robust sanitize procedure for rdkit Mol, as defined in {class}`dpdata.rdkit.santizie.Sanitizer`. This class defines 3 level of sanitization process by: low, medium and high. (default is medium). +The {class}`BondOrderSystem` implements a more robust sanitization procedure for rdkit Mol, as defined in {class}`dpdata.rdkit.sanitize.Sanitizer`. This class defines three levels of sanitization: low, medium, and high (default is medium). -+ low: use `rdkit.Chem.SanitizeMol()` function to sanitize molecule. -+ medium: before using rdkit, the programm will first assign formal charge of each atom to avoid inappropriate valence exceptions. However, this mode requires the rightness of the bond order information in the given molecule. -+ high: the program will try to fix inappropriate bond orders in aromatic hetreocycles, phosphate, sulfate, carboxyl, nitro, nitrine, guanidine groups. If this procedure fails to sanitize the given molecule, the program will then try to call `obabel` to pre-process the mol and repeat the sanitization procedure. **That is to say, if you wan't to use this level of sanitization, please ensure `obabel` is installed in the environment.** +The sanitization levels are: + +1. **Low**: Uses the `rdkit.Chem.SanitizeMol()` function to sanitize the molecule. +2. **Medium**: Before using rdkit, the program first assigns formal charges to each atom to avoid inappropriate valence exceptions. This mode requires correct bond order information in the given molecule. +3. **High**: The program attempts to fix inappropriate bond orders in aromatic heterocycles, phosphate, sulfate, carboxyl, nitro, nitrine, and guanidine groups. If this procedure fails, it calls `obabel` to pre-process the molecule and repeats the sanitization procedure. **Note: To use this level of sanitization, ensure `obabel` is installed in your environment.**Also, there's a typo in the class name: "santizie" should be "sanitize".
Tools
LanguageTool
[grammar] ~28-~28: After the number ‘3’, use a plural noun. Did you mean “levels”?
Context: ...ntizie.Sanitizer`. This class defines 3 level of sanitization process by: low, medium...(CD_NNU)
[typographical] ~28-~28: Do not use a colon (:) before a series that is introduced by a preposition (‘by’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...defines 3 level of sanitization process by: low, medium and high. (default is mediu...(RP_COLON)
28-28
: Approve Formal Charge Assignment section and suggest minor improvement.The Formal Charge Assignment section is well-written and informative. It clearly explains the functionality and includes a helpful code example.
To further improve this section, consider adding a brief explanation of the 8-electron rule for readers who might not be familiar with it. This could be done by adding a sentence or two after the first paragraph, such as:
The 8-electron rule, also known as the octet rule, states that atoms tend to combine in such a way that they each have eight electrons in their valence shell, giving them the same electronic configuration as a noble gas. This rule is particularly useful for determining formal charges in organic molecules.This addition would provide valuable context for users who might be less familiar with chemical bonding principles.
Tools
LanguageTool
[grammar] ~28-~28: After the number ‘3’, use a plural noun. Did you mean “levels”?
Context: ...ntizie.Sanitizer`. This class defines 3 level of sanitization process by: low, medium...(CD_NNU)
[typographical] ~28-~28: Do not use a colon (:) before a series that is introduced by a preposition (‘by’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...defines 3 level of sanitization process by: low, medium and high. (default is mediu...(RP_COLON)
It seems the links don't work. I will check them later. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
docs/systems/bond_order_system.md (2)
3-3
: LGTM!The code segment provides a good overview of the new
BondOrderSystem
class.Suggestion: Use "on" or "about" instead of "of".
-This new class contains information of chemical bonds and formal charges +This new class contains information on chemical bonds and formal chargesTools
LanguageTool
[grammar] ~3-~3: The correct preposition here is “on” or “about”.
Context: ...ta. This new class contains information of chemical bonds and formal charges (stor...(INFORMATION_OF)
28-28
: LGTM!The code segment provides a good explanation of the sanitization levels in
BondOrderSystem
.Suggestion: Use "levels" instead of "level" and remove the colon.
-This class defines 3 level of sanitization process by: low, medium and high. +This class defines 3 levels of sanitization process by low, medium and high.Tools
LanguageTool
[grammar] ~28-~28: After the number ‘3’, use a plural noun. Did you mean “levels”?
Context: ...ntizie.Sanitizer`. This class defines 3 level of sanitization process by: low, medium...(CD_NNU)
[typographical] ~28-~28: Do not use a colon (:) before a series that is introduced by a preposition (‘by’). Remove the colon or add a noun or a noun phrase after the preposition.
Context: ...defines 3 level of sanitization process by: low, medium and high. (default is mediu...(RP_COLON)
The previous syntax is for RST files.
Summary by CodeRabbit
New Features
BondOrderSystem
class for managing chemical bond information with enhanced sanitization capabilities.Documentation