Copy matgenb repo into new pymatgen examples/ folder
#3811
Conversation
…ew pymatgen examples/ folder
janosh
added
docs
|
Note Reviews PausedUse the following commands to manage reviews:
WalkthroughThis batch of updates introduces a variety of new functionalities and enhancements across several Jupyter notebooks and data files within the pymatgen library. Key additions include tools for calculating reaction energies, retrieving crystal structures, managing units, predicting structures, and identifying coordination environments. The updates also extend to several configuration and data files, improving the library's integration with computational tools and databases. Changes
This summary provides a concise overview of significant changes, focusing on new functionalities and enhancements in the pymatgen library and associated examples. 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 as PR comments)
Additionally, you can add CodeRabbit Configration 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: 36
Out of diff range and nitpick comments (6)
examples/README.md (2)
14-14: Consider using a hyphen in "open-source" when used as a compound adjective.
75-75: Consider using a more formal alternative to "Feel free to" to enhance the professionalism of the document.examples/lobster_data/GaAs/IBZKPT (1)
1-1: Ensure the header comment accurately describes the file's purpose and contents.examples/2019-03-11-Interface Reactions.ipynb (2)
85-128: The logic for handling both open and closed systems in the reaction interface is well-implemented. However, consider adding comments to explain the significance of each step, especially for complex operations like building phase diagrams and handling chemical potentials.
155-156: Ensure that the variablepltis used after assignment to display or manipulate the plot, or consider removing the assignment if it is unnecessary.examples/2013-01-01-Plotting and Analyzing a Phase Diagram using the Materials API.ipynb (1)
49-57: Consider adding a note about setting thePMG_MAPI_KEYenvironment variable for theMPRester()API key.
| "critical_rxns = [\n", | ||
| " OrderedDict(\n", | ||
| " [\n", | ||
| " (\"Atomic fraction\", round(ratio, 3)),\n", | ||
| " (\"Reaction equation\", rxn),\n", | ||
| " (\"E$_{rxt}$ per mol equation (kJ/mol)\", round(rxn_energy, 1)),\n", | ||
| " (\"E$_{rxt}$ per reactant atom (eV/atom)\", round(reactivity, 3)),\n", | ||
| " ]\n", | ||
| " )\n", | ||
| " for _, ratio, reactivity, rxn, rxn_energy in interface.get_kinks()\n", | ||
| "]\n", | ||
| "interface_reaction_table = pd.DataFrame(critical_rxns)\n", | ||
| "interface_reaction_table" |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has confirmed that there are no try-except blocks implemented in the 2019-03-11-Interface Reactions.ipynb notebook for handling potential exceptions from external API calls or data processing. This aligns with the initial review comment suggesting the addition of error handling mechanisms.
- File: examples/2019-03-11-Interface Reactions.ipynb
- Recommendation: Consider implementing error handling to manage exceptions that may arise during external API interactions or data processing to enhance the robustness of the notebook.
Analysis chain
The use of OrderedDict and list comprehensions for creating data frames is correct. However, consider adding error handling to manage potential exceptions from external API calls or data processing.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that error handling is implemented for external API calls and data processing.
# Test: Search for try-except blocks around external API calls and data processing.
rg --type py 'try:.*?except' '2019-03-11-Interface Reactions.ipynb'
Length of output: 146
Script:
#!/bin/bash
# Description: Search for the notebook file to confirm its presence and correct path.
fd '2019-03-11-Interface Reactions.ipynb'
Length of output: 89
Script:
#!/bin/bash
# Description: Verify that error handling is implemented for external API calls and data processing in the correct notebook file path.
# Test: Search for try-except blocks around external API calls and data processing.
rg --type py 'try:.*?except' 'examples/2019-03-11-Interface Reactions.ipynb'
Length of output: 78
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | ||
| "# !pip install pymatgen==2022.7.19" |
There was a problem hiding this comment.
Consider adding a conditional check to ensure that the installation of dependencies is only performed if the notebook is run in Google Colab. This prevents unnecessary installations when the notebook is run in other environments.
+ if 'google.colab' in str(get_ipython()):
- # Uncomment the subsequent lines in this cell to install dependencies for Google Colab.
- # !pip install pymatgen==2022.7.19
+ !pip install pymatgen==2022.7.19Committable 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.
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | |
| "# !pip install pymatgen==2022.7.19" | |
| if 'google.colab' in str(get_ipython()): | |
| !pip install pymatgen==2022.7.19 |
| } | ||
| ], | ||
| "source": [ | ||
| "entries[25].energy_adjustments" |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification script has confirmed that the entries list has a length of 0, which means that accessing the index 25, as mentioned in the code snippet, will indeed result in an IndexError. This confirms the validity of the initial review comment regarding the potential for an error due to an out-of-bounds index.
Action Required:
- The code snippet attempting to access
entries[25]needs to be revised to ensure that it does not attempt to access an index that is out of bounds. This could involve checking the length of the list before attempting to access this index, or handling the potentialIndexErrorin a way that does not disrupt the program's execution.
Locations to update:
examples/2021-5-12-Explanation of Corrections.ipynbat the line whereentries[25]is accessed.
Analysis chain
Ensure that the index used (25) is within the bounds of the entries list to prevent potential IndexError.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the index 25 is within the bounds of the entries list.
# Test: Check the length of the entries list. Expect: Length greater than 25.
echo "Length of entries list: ${#entries[@]}"
Length of output: 60
| @@ -0,0 +1,944 @@ | |||
| \Gamma -> X -> W -> K -> \Gamma -> L -> U -> W -> L -> K -> U -> X | |||
| 941 | |||
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has revealed a discrepancy between the number of k-points specified on line 2 of the file examples/lobster_data/GaAs/KPOINTS and the actual number of k-point entries present in the file. The specified number is 941, while the actual count is 750.
Action Required
- Please review the
KPOINTSfile to ensure the specified number of k-points matches the actual entries. This may involve either updating the number on line 2 or adjusting the k-point entries in the file.
Analysis chain
Confirm the total number of k-points listed matches the number specified on line 2.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Ensure the total number of k-points matches the number specified.
# Count the number of k-point entries and compare with the number on line 2.
total_kpoints=$(grep -c '^[0-9]' examples/lobster_data/GaAs/KPOINTS)
specified_kpoints=$(sed -n '2p' examples/lobster_data/GaAs/KPOINTS)
if [[ "$total_kpoints" -eq "$specified_kpoints" ]]; then
echo "K-point count matches."
else
echo "Mismatch in k-point count. Expected $specified_kpoints, found $total_kpoints."
fi
Length of output: 279
| 0.5 0.324468085106383 0.675531914893617 0 | ||
| 0.5 0.32978723404255317 0.6702127659574468 0 | ||
| 0.5 0.3351063829787234 0.6648936170212766 0 | ||
| 0.5 0.3404255319148936 0.6595744680851063 0 | ||
| 0.5 0.3457446808510638 0.6542553191489361 0 | ||
| 0.5 0.351063829787234 0.648936170212766 0 | ||
| 0.5 0.35638297872340424 0.6436170212765957 0 | ||
| 0.5 0.36170212765957444 0.6382978723404256 0 | ||
| 0.5 0.3670212765957447 0.6329787234042553 0 | ||
| 0.5 0.3723404255319149 0.6276595744680851 0 | ||
| 0.5 0.37765957446808507 0.6223404255319149 0 | ||
| 0.5 0.3829787234042553 0.6170212765957447 0 | ||
| 0.5 0.38829787234042556 0.6117021276595744 0 | ||
| 0.5 0.39361702127659576 0.6063829787234043 0 | ||
| 0.5 0.39893617021276595 0.601063829787234 0 | ||
| 0.5 0.4042553191489362 0.5957446808510638 0 | ||
| 0.5 0.4095744680851064 0.5904255319148937 0 | ||
| 0.5 0.4148936170212766 0.5851063829787235 0 | ||
| 0.5 0.42021276595744683 0.5797872340425532 0 | ||
| 0.5 0.425531914893617 0.574468085106383 0 | ||
| 0.5 0.4308510638297872 0.5691489361702127 0 | ||
| 0.5 0.43617021276595747 0.5638297872340425 0 | ||
| 0.5 0.44148936170212766 0.5585106382978724 0 | ||
| 0.5 0.44680851063829785 0.553191489361702 0 | ||
| 0.5 0.4521276595744681 0.5478723404255319 0 | ||
| 0.5 0.4574468085106383 0.5425531914893618 0 | ||
| 0.5 0.4627659574468085 0.5372340425531915 0 | ||
| 0.5 0.46808510638297873 0.5319148936170213 0 | ||
| 0.5 0.47340425531914887 0.5265957446808511 0 | ||
| 0.5 0.4787234042553191 0.5212765957446809 0 | ||
| 0.5 0.48404255319148937 0.5159574468085106 0 | ||
| 0.5 0.48936170212765956 0.5106382978723405 0 | ||
| 0.49999999999999994 0.49468085106382975 0.5053191489361701 0 | ||
| 0.5 0.5 0.5 0 L | ||
| 0.4969512195121951 0.4969512195121951 0.5060975609756098 0 | ||
| 0.49390243902439024 0.49390243902439024 0.5121951219512195 0 | ||
| 0.49085365853658536 0.49085365853658536 0.5182926829268293 0 | ||
| 0.48780487804878053 0.48780487804878053 0.524390243902439 0 | ||
| 0.4847560975609756 0.4847560975609756 0.5304878048780488 0 | ||
| 0.4817073170731707 0.4817073170731707 0.5365853658536586 0 | ||
| 0.4786585365853659 0.4786585365853659 0.5426829268292683 0 | ||
| 0.475609756097561 0.475609756097561 0.5487804878048781 0 | ||
| 0.4725609756097561 0.4725609756097561 0.5548780487804877 0 | ||
| 0.46951219512195125 0.46951219512195125 0.5609756097560976 0 | ||
| 0.4664634146341464 0.4664634146341464 0.5670731707317074 0 | ||
| 0.46341463414634143 0.46341463414634143 0.573170731707317 0 | ||
| 0.46036585365853655 0.46036585365853655 0.5792682926829268 0 | ||
| 0.4573170731707318 0.4573170731707318 0.5853658536585367 0 | ||
| 0.45426829268292684 0.45426829268292684 0.5914634146341463 0 | ||
| 0.4512195121951219 0.4512195121951219 0.5975609756097561 0 | ||
| 0.44817073170731714 0.44817073170731714 0.603658536585366 0 | ||
| 0.4451219512195122 0.4451219512195122 0.6097560975609756 0 | ||
| 0.44207317073170727 0.44207317073170727 0.6158536585365854 0 | ||
| 0.43902439024390244 0.43902439024390244 0.6219512195121951 0 | ||
| 0.43597560975609756 0.43597560975609756 0.6280487804878049 0 | ||
| 0.4329268292682927 0.4329268292682927 0.6341463414634146 0 | ||
| 0.4298780487804878 0.4298780487804878 0.6402439024390244 0 | ||
| 0.42682926829268286 0.42682926829268286 0.646341463414634 0 | ||
| 0.4237804878048781 0.4237804878048781 0.6524390243902439 0 | ||
| 0.4207317073170732 0.4207317073170732 0.6585365853658537 0 | ||
| 0.4176829268292682 0.4176829268292682 0.6646341463414633 0 | ||
| 0.41463414634146345 0.41463414634146345 0.6707317073170732 0 | ||
| 0.41158536585365857 0.41158536585365857 0.676829268292683 0 | ||
| 0.40853658536585363 0.40853658536585363 0.6829268292682926 0 | ||
| 0.40548780487804875 0.40548780487804875 0.6890243902439024 0 | ||
| 0.4024390243902439 0.4024390243902439 0.6951219512195123 0 | ||
| 0.399390243902439 0.399390243902439 0.7012195121951219 0 | ||
| 0.3963414634146341 0.3963414634146341 0.7073170731707317 0 | ||
| 0.3932926829268293 0.3932926829268293 0.7134146341463414 0 | ||
| 0.3902439024390244 0.3902439024390244 0.7195121951219512 0 | ||
| 0.38719512195121947 0.38719512195121947 0.725609756097561 0 | ||
| 0.38414634146341464 0.38414634146341464 0.7317073170731708 0 | ||
| 0.38109756097560976 0.38109756097560976 0.7378048780487805 0 | ||
| 0.3780487804878049 0.3780487804878049 0.7439024390243902 0 | ||
| 0.375 0.375 0.75 0 K | ||
| 0.625 0.25 0.625 0 U | ||
| 0.6197916666666666 0.23958333333333334 0.6197916666666666 0 | ||
| 0.6145833333333334 0.22916666666666666 0.6145833333333334 0 | ||
| 0.609375 0.21875 0.609375 0 | ||
| 0.6041666666666666 0.20833333333333334 0.6041666666666666 0 | ||
| 0.5989583333333334 0.19791666666666669 0.5989583333333334 0 | ||
| 0.59375 0.1875 0.59375 0 | ||
| 0.5885416666666666 0.17708333333333331 0.5885416666666666 0 | ||
| 0.5833333333333334 0.16666666666666669 0.5833333333333334 0 | ||
| 0.578125 0.15625 0.578125 0 | ||
| 0.5729166666666666 0.14583333333333334 0.5729166666666666 0 | ||
| 0.5677083333333334 0.13541666666666669 0.5677083333333334 0 | ||
| 0.5625 0.125 0.5625 0 | ||
| 0.5572916666666666 0.11458333333333336 0.5572916666666666 0 | ||
| 0.5520833333333334 0.10416666666666664 0.5520833333333334 0 | ||
| 0.546875 0.09375 0.546875 0 | ||
| 0.5416666666666666 0.08333333333333334 0.5416666666666666 0 | ||
| 0.5364583333333334 0.07291666666666667 0.5364583333333334 0 | ||
| 0.53125 0.06250000000000001 0.53125 0 | ||
| 0.5260416666666666 0.052083333333333336 0.5260416666666666 0 | ||
| 0.5208333333333334 0.04166666666666666 0.5208333333333334 0 | ||
| 0.515625 0.03125000000000001 0.515625 0 | ||
| 0.5104166666666666 0.020833333333333356 0.5104166666666666 0 | ||
| 0.5052083333333334 0.010416666666666678 0.5052083333333334 0 | ||
| 0.5 0.0 0.5 0 X |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has identified multiple issues in the KPOINTS file regarding the range and format of k-point coordinates. Here are the details:
- Range Issues: Several k-point coordinates exceed the expected range of -0.5 to 0.5. This is evident from lines where coordinates like
0.5075757575757576,0.75, etc., appear. - Format Issues: The expected format of four columns per line is not consistently followed. This is particularly noticeable in lines where labels or comments are present, such as line 1 or lines with
\Gamma,X,W,K,L,U.
Given these findings, it is recommended to review and correct the k-point entries in the file to ensure they are within the correct range and adhere to the expected format.
Analysis chain
Ensure all k-point coordinates are within the expected range for the reciprocal lattice and that the format (four columns per line) is consistent throughout the file.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check that all k-point coordinates are within the expected range and format.
# Test: Verify the range and format of k-point entries.
awk '{if (NF != 4 || $1 < -0.5 || $1 > 0.5 || $2 < -0.5 || $2 > 0.5 || $3 < -0.5 || $3 > 0.5) print "Error in line " NR ": " $0}' examples/lobster_data/GaAs/KPOINTS
Length of output: 18187
| "output_type": "stream", | ||
| "text": [ | ||
| "The force is 19.62 N\n", | ||
| "The potential energy is force is 196.20000000000002 J\n" |
There was a problem hiding this comment.
Correct the typo in the output text for better clarity.
- "print(f\"The potential energy is force is {(m * g * h).to('J')}\")"
+ "print(f\"The potential energy is {(m * g * h).to('J')}\")"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.
| "The potential energy is force is 196.20000000000002 J\n" | |
| "The potential energy is 196.20000000000002 J\n" |
| "# Introduction\n", | ||
| "\n", | ||
| "This notebook demonstrates how to carry out an ordering of a disordered structure using pymatgen." | ||
| ] |
There was a problem hiding this comment.
Ensure the introductory markdown clearly explains the purpose and scope of the notebook.
- "# Introduction\n",
- "\n",
- "This notebook demonstrates how to carry out an ordering of a disordered structure using pymatgen."
+ "# Introduction to Ordering Disordered Structures in Pymatgen\n",
+ "This notebook provides a comprehensive guide on how to order disordered structures using various transformations available in pymatgen, aimed at users interested in materials science simulations."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.
| "# Introduction\n", | |
| "\n", | |
| "This notebook demonstrates how to carry out an ordering of a disordered structure using pymatgen." | |
| ] | |
| "# Introduction to Ordering Disordered Structures in Pymatgen\n", | |
| "This notebook provides a comprehensive guide on how to order disordered structures using various transformations available in pymatgen, aimed at users interested in materials science simulations." |
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | ||
| "# !pip install pymatgen==2022.7.19" | ||
| ] |
There was a problem hiding this comment.
Check the necessity of the commented installation command for dependencies.
- "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n",
- "# !pip install pymatgen==2022.7.19"
+ "# If running in Google Colab, uncomment the following line to install the required pymatgen version.\n",
+ "# !pip install pymatgen==2022.7.19"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.
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | |
| "# !pip install pymatgen==2022.7.19" | |
| ] | |
| "# If running in Google Colab, uncomment the following line to install the required pymatgen version.\n", | |
| "# !pip install pymatgen==2022.7.19" |
| "About the `matbench_dielectric` dataset:\n", | ||
| "\n", | ||
| "Intended use: Machine learning task to predict refractive index from structure.\n", | ||
| " All data from Materials Project. Removed entries having a formation energy (or energy\n", | ||
| " above the convex hull) more than 150meV and those having refractive indices less than\n", | ||
| " 1 and those containing noble gases. Retrieved April 2, 2019.\n", | ||
| "Input: Pymatgen Structure of the material\n", | ||
| "Target variable: refractive index n (unitless)\n", | ||
| "Entries: 636\n", | ||
| "URL: https://ml.materialsproject.org/projects/matbench_dielectric\n", | ||
| "\n", | ||
| "Here's what the Dash server spun up in the last cell should look like:\n", | ||
| "\n", | ||
| "\n", | ||
| "\n", | ||
| "\n", | ||
| "Requires installing [`crystal-toolkit`](https://github.com/materialsproject/crystaltoolkit) and [`dash`](https://github.com/plotly/dash)." |
There was a problem hiding this comment.
Ensure the introductory markdown clearly explains the purpose and scope of the notebook and the dataset used.
- "About the `matbench_dielectric` dataset:\n",
- "\n",
- "Intended use: Machine learning task to predict refractive index from structure.\n",
- " All data from Materials Project. Removed entries having a formation energy (or energy\n",
- " above the convex hull) more than 150meV and those having refractive indices less than\n",
- " 1 and those containing noble gases. Retrieved April 2, 2019.\n",
- "Input: Pymatgen Structure of the material\n",
- "Target variable: refractive index n (unitless)\n",
- "Entries: 636\n",
- "URL: https://ml.materialsproject.org/projects/matbench_dielectric\n",
- "\n",
- "Here's what the Dash server spun up in the last cell should look like:\n",
- "\n",
- "\n",
- "\n",
- "\n",
- "Requires installing [`crystal-toolkit`](https://github.com/materialsproject/crystaltoolkit) and [`dash`](https://github.com/plotly/dash)."
+ "## About the `matbench_dielectric` dataset\n",
+ "\n",
+ "**Intended use**: This dataset is used for a machine learning task to predict the refractive index from the structure of materials. All data is sourced from the Materials Project.\n",
+ "\n",
+ "**Criteria for data selection**:\n",
+ "- Formation energy (or energy above the convex hull) less than 150 meV.\n",
+ "- Refractive indices greater than 1.\n",
+ "- Excludes entries containing noble gases.\n",
+ "\n",
+ "**Dataset details**:\n",
+ "- Input: Pymatgen Structure of the material.\n",
+ "- Target variable: Refractive index (n), unitless.\n",
+ "- Number of entries: 636.\n",
+ "- Data retrieval date: April 2, 2019.\n",
+ "- Dataset URL: [Matbench Dielectric](https://ml.materialsproject.org/projects/matbench_dielectric).\n",
+ "\n",
+ "Below is an example of what the Dash server, set up in the last cell, should display:\n",
+ "\n",
+ "\n",
+ "\n",
+ "**Dependencies**:\n",
+ "- [`crystal-toolkit`](https://github.com/materialsproject/crystaltoolkit)\n",
+ "- [`dash`](https://github.com/plotly/dash)\n",
+ "These libraries are required to run the interactive structure viewer."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.
| "About the `matbench_dielectric` dataset:\n", | |
| "\n", | |
| "Intended use: Machine learning task to predict refractive index from structure.\n", | |
| " All data from Materials Project. Removed entries having a formation energy (or energy\n", | |
| " above the convex hull) more than 150meV and those having refractive indices less than\n", | |
| " 1 and those containing noble gases. Retrieved April 2, 2019.\n", | |
| "Input: Pymatgen Structure of the material\n", | |
| "Target variable: refractive index n (unitless)\n", | |
| "Entries: 636\n", | |
| "URL: https://ml.materialsproject.org/projects/matbench_dielectric\n", | |
| "\n", | |
| "Here's what the Dash server spun up in the last cell should look like:\n", | |
| "\n", | |
| "\n", | |
| "\n", | |
| "\n", | |
| "Requires installing [`crystal-toolkit`](https://github.com/materialsproject/crystaltoolkit) and [`dash`](https://github.com/plotly/dash)." | |
| "## About the `matbench_dielectric` dataset\n", | |
| "\n", | |
| "**Intended use**: This dataset is used for a machine learning task to predict the refractive index from the structure of materials. All data is sourced from the Materials Project.\n", | |
| "\n", | |
| "**Criteria for data selection**:\n", | |
| "- Formation energy (or energy above the convex hull) less than 150 meV.\n", | |
| "- Refractive indices greater than 1.\n", | |
| "- Excludes entries containing noble gases.\n", | |
| "\n", | |
| "**Dataset details**:\n", | |
| "- Input: Pymatgen Structure of the material.\n", | |
| "- Target variable: Refractive index (n), unitless.\n", | |
| "- Number of entries: 636.\n", | |
| "- Data retrieval date: April 2, 2019.\n", | |
| "- Dataset URL: [Matbench Dielectric](https://ml.materialsproject.org/projects/matbench_dielectric).\n", | |
| "\n", | |
| "Below is an example of what the Dash server, set up in the last cell, should display:\n", | |
| "\n", | |
| "\n", | |
| "\n", | |
| "**Dependencies**:\n", | |
| "- [`crystal-toolkit`](https://github.com/materialsproject/crystaltoolkit)\n", | |
| "- [`dash`](https://github.com/plotly/dash)\n", | |
| "These libraries are required to run the interactive structure viewer." |
| "metadata": {}, | ||
| "outputs": [], | ||
| "source": [ | ||
| "!pip install dash crystal-toolkit" |
There was a problem hiding this comment.
Ensure the installation command is necessary and correctly formatted.
- "!pip install dash crystal-toolkit"
+ "# Install dash and crystal-toolkit using the following command:\n",
+ "!pip install dash crystal-toolkit"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.
| "!pip install dash crystal-toolkit" | |
| "# Install dash and crystal-toolkit using the following command:\n", | |
| "!pip install dash crystal-toolkit" |
|
@coderabbitai ignore |
There was a problem hiding this comment.
Actionable comments posted: 12
Out of diff range and nitpick comments (4)
examples/2019-03-11-Interface Reactions.ipynb (2)
1-12: Ensure the introductory markdown cell clearly outlines the notebook's objectives and prerequisites.Consider enhancing the introductory markdown cell to include a brief overview of the prerequisites (e.g., necessary library installations) and the objectives of the notebook. This will help users understand the purpose and requirements of the notebook at a glance.
86-130: Ensure that the phase diagram and interface reactivity calculations are correctly implemented and documented.Consider adding detailed comments explaining each step of the phase diagram and interface reactivity calculations. This will improve the maintainability and understandability of the code.
examples/2018-03-09-Computing the Reaction Diagram between Two Compounds.ipynb (1)
361-366: Ensure that the loop variableiis used if it's necessary, or replace it with_if it's unused to indicate that it's a placeholder.examples/2017-04-03-Slab generation and Wulff shape.ipynb (1)
324-326: Clarify the comment about the bond length condition inget_slabs.Consider rephrasing to explicitly state that the bond length condition is a constraint that must not be exceeded when generating slabs.
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | ||
| "# !pip install pymatgen==2022.7.19" | ||
| ] |
There was a problem hiding this comment.
Consider adding a conditional check to ensure this notebook is being run in Google Colab before attempting to install packages.
+ if 'google.colab' in str(get_ipython()):
!pip install pymatgen==2022.7.19Committable 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.
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | |
| "# !pip install pymatgen==2022.7.19" | |
| ] | |
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | |
| "if 'google.colab' in str(get_ipython()):\n", | |
| " !pip install pymatgen==2022.7.19" |
| "from pymatgen.analysis.phase_diagram import PhaseDiagram, ReactionDiagram\n", | ||
| "from pymatgen.entries.compatibility import MaterialsProjectCompatibility\n", | ||
| "from pymatgen.ext.matproj import MPRester\n", | ||
| "\n", | ||
| "%matplotlib inline" |
There was a problem hiding this comment.
Reorder the import statements to follow PEP 8 guidelines: standard library imports first, followed by third-party imports, and application-specific imports last.
- from pymatgen.analysis.phase_diagram import PhaseDiagram, ReactionDiagram
- from pymatgen.entries.compatibility import MaterialsProjectCompatibility
- from pymatgen.ext.matproj import MPRester
-
- %matplotlib inline
+ %matplotlib inline
+ from pymatgen.analysis.phase_diagram import PhaseDiagram, ReactionDiagram
+ from pymatgen.entries.compatibility import MaterialsProjectCompatibility
+ from pymatgen.ext.matproj import MPResterCommittable 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.
| "from pymatgen.analysis.phase_diagram import PhaseDiagram, ReactionDiagram\n", | |
| "from pymatgen.entries.compatibility import MaterialsProjectCompatibility\n", | |
| "from pymatgen.ext.matproj import MPRester\n", | |
| "\n", | |
| "%matplotlib inline" | |
| "%matplotlib inline\n", | |
| "from pymatgen.analysis.phase_diagram import PhaseDiagram, ReactionDiagram\n", | |
| "from pymatgen.entries.compatibility import MaterialsProjectCompatibility\n", | |
| "from pymatgen.ext.matproj import MPRester\n", |
| "# Specify location of CIF on your device\n", | ||
| "structure = Structure.from_file(\"/home/user/Documents/cifs/lfp.cif\")\n", | ||
| "print(structure)" |
There was a problem hiding this comment.
Consider using a relative path or adding instructions for users to set their file paths.
- structure = Structure.from_file("/home/user/Documents/cifs/lfp.cif")
+ # Adjust the path as per your local setup
+ structure = Structure.from_file("path_to_your_cif_file/lfp.cif")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.
| "# Specify location of CIF on your device\n", | |
| "structure = Structure.from_file(\"/home/user/Documents/cifs/lfp.cif\")\n", | |
| "print(structure)" | |
| "# Specify location of CIF on your device\n", | |
| "# Adjust the path as per your local setup\n", | |
| "structure = Structure.from_file(\"path_to_your_cif_file/lfp.cif\")\n", | |
| "print(structure)" |
| "# create the directory \"fermi/vbm\" before\n", | ||
| "os.makedirs(\"fermi/vbm/\", exist_ok=True)\n", | ||
| "\n", | ||
| "# run the interpolation, lpfac should be between 50 and 150\n", |
There was a problem hiding this comment.
The lpfac parameter is set to 10, which is below the recommended range of 50 to 150. Adjust this to ensure optimal performance.
- BoltztrapRunner(bs=bs, nelec=nelect, lpfac=10, run_type="FERMI", band_nb=vbm, cond_band=False).run(
+ BoltztrapRunner(bs=bs, nelec=nelect, lpfac=50, run_type="FERMI", band_nb=vbm, cond_band=False).run(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.
| "# run the interpolation, lpfac should be between 50 and 150\n", | |
| "# run the interpolation, lpfac should be between 50 and 150\n", |
| "# create the directory \"fermi/cbm\" before\n", | ||
| "os.makedirs(\"fermi/cbm/\", exist_ok=True)\n", | ||
| "\n", | ||
| "# run the interpolation, lpfac should be between 50 and 150\n", |
There was a problem hiding this comment.
The lpfac parameter is incorrectly set to 10 here as well. It should be within the range of 50 to 150.
- BoltztrapRunner(bs=bs, nelec=nelect, lpfac=10, run_type="FERMI", band_nb=cbm, cond_band=True).run(path_dir="fermi/cbm/")
+ BoltztrapRunner(bs=bs, nelec=nelect, lpfac=50, run_type="FERMI", band_nb=cbm, cond_band=True).run(path_dir="fermi/cbm/")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.
| "# run the interpolation, lpfac should be between 50 and 150\n", | |
| "# run the interpolation, lpfac should be between 50 and 150\n", | |
| BoltztrapRunner(bs=bs, nelec=nelect, lpfac=50, run_type="FERMI", band_nb=cbm, cond_band=True).run(path_dir="fermi/cbm/") |
| "surface_energies_Ni = {\n", | ||
| " (3, 2, 0): 2.3869,\n", | ||
| " (1, 1, 0): 2.2862,\n", | ||
| " (3, 1, 0): 2.3964,\n", | ||
| " (2, 1, 0): 2.3969,\n", | ||
| " (3, 3, 2): 2.0944,\n", | ||
| " (1, 0, 0): 2.2084,\n", | ||
| " (2, 1, 1): 2.2353,\n", | ||
| " (3, 2, 2): 2.1242,\n", | ||
| " (3, 2, 1): 2.3183,\n", | ||
| " (2, 2, 1): 2.1732,\n", | ||
| " (3, 3, 1): 2.2288,\n", | ||
| " (3, 1, 1): 2.3039,\n", | ||
| " (1, 1, 1): 1.9235,\n", | ||
| "}\n", |
There was a problem hiding this comment.
Ensure the dictionary keys and values are correctly aligned for better readability.
- "surface_energies_Ni = {\n",
- " (3, 2, 0): 2.3869,\n",
- " (1, 1, 0): 2.2862,\n",
- " (3, 1, 0): 2.3964,\n",
- " (2, 1, 0): 2.3969,\n",
- " (3, 3, 2): 2.0944,\n",
- " (1, 0, 0): 2.2084,\n",
- " (2, 1, 1): 2.2353,\n",
- " (3, 2, 2): 2.1242,\n",
- " (3, 2, 1): 2.3183,\n",
- " (2, 2, 1): 2.1732,\n",
- " (3, 3, 1): 2.2288,\n",
- " (3, 1, 1): 2.3039,\n",
- " (1, 1, 1): 1.9235,\n",
- "}\n",
+ "surface_energies_Ni = {\n",
+ " (3, 2, 0): 2.3869,\n",
+ " (1, 1, 0): 2.2862,\n",
+ " (3, 1, 0): 2.3964,\n",
+ " (2, 1, 0): 2.3969,\n",
+ " (3, 3, 2): 2.0944,\n",
+ " (1, 0, 0): 2.2084,\n",
+ " (2, 1, 1): 2.2353,\n",
+ " (3, 2, 2): 2.1242,\n",
+ " (3, 2, 1): 2.3183,\n",
+ " (2, 2, 1): 2.1732,\n",
+ " (3, 3, 1): 2.2288,\n",
+ " (3, 1, 1): 2.3039,\n",
+ " (1, 1, 1): 1.9235,\n",
+ "}\n",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.
| "surface_energies_Ni = {\n", | |
| " (3, 2, 0): 2.3869,\n", | |
| " (1, 1, 0): 2.2862,\n", | |
| " (3, 1, 0): 2.3964,\n", | |
| " (2, 1, 0): 2.3969,\n", | |
| " (3, 3, 2): 2.0944,\n", | |
| " (1, 0, 0): 2.2084,\n", | |
| " (2, 1, 1): 2.2353,\n", | |
| " (3, 2, 2): 2.1242,\n", | |
| " (3, 2, 1): 2.3183,\n", | |
| " (2, 2, 1): 2.1732,\n", | |
| " (3, 3, 1): 2.2288,\n", | |
| " (3, 1, 1): 2.3039,\n", | |
| " (1, 1, 1): 1.9235,\n", | |
| "}\n", | |
| "surface_energies_Ni = {\n", | |
| " (3, 2, 0): 2.3869,\n", | |
| " (1, 1, 0): 2.2862,\n", | |
| " (3, 1, 0): 2.3964,\n", | |
| " (2, 1, 0): 2.3969,\n", | |
| " (3, 3, 2): 2.0944,\n", | |
| " (1, 0, 0): 2.2084,\n", | |
| " (2, 1, 1): 2.2353,\n", | |
| " (3, 2, 2): 2.1242,\n", | |
| " (3, 2, 1): 2.3183,\n", | |
| " (2, 2, 1): 2.1732,\n", | |
| " (3, 3, 1): 2.2288,\n", | |
| " (3, 1, 1): 2.3039,\n", | |
| " (1, 1, 1): 1.9235,\n", | |
| "}\n", |
| "# Uncomment the subsequent lines in this cell to install dependencies for Google Colab.\n", | ||
| "# !pip install pymatgen==2022.7.19" |
There was a problem hiding this comment.
Consider adding a conditional check to ensure the notebook is running in Google Colab before attempting to install packages.
Would you like me to generate the code to add this conditional check, or should I open a GitHub issue to track this enhancement?
| "# Is the system open to an elemental reservoir?\n", | ||
| "grand = True\n", | ||
| "\n", | ||
| "if grand:\n", | ||
| " # Element in the elemental reservoir.\n", | ||
| " open_el = \"Co\"\n", | ||
| " # Relative chemical potential vs. pure substance. Must be non-positive.\n", | ||
| " relative_mu = -1" | ||
| ] |
There was a problem hiding this comment.
Optimize the handling of the system's openness to an elemental reservoir.
- grand = True
- if grand:
+ open_system = True
+ if open_system:Refactor the variable name for clarity and directness in indicating the system's state.
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.
| "# Is the system open to an elemental reservoir?\n", | |
| "grand = True\n", | |
| "\n", | |
| "if grand:\n", | |
| " # Element in the elemental reservoir.\n", | |
| " open_el = \"Co\"\n", | |
| " # Relative chemical potential vs. pure substance. Must be non-positive.\n", | |
| " relative_mu = -1" | |
| ] | |
| "# Is the system open to an elemental reservoir?\n", | |
| "open_system = True\n", | |
| "\n", | |
| "if open_system:\n", | |
| " # Element in the elemental reservoir.\n", | |
| " open_el = \"Co\"\n", | |
| " # Relative chemical potential vs. pure substance. Must be non-positive.\n", | |
| " relative_mu = -1" | |
| ] |
| "from collections import OrderedDict\n", | ||
| "\n", | ||
| "import pandas as pd\n", | ||
| "\n", | ||
| "critical_rxns = [\n", | ||
| " OrderedDict(\n", | ||
| " [\n", | ||
| " (\"Atomic fraction\", round(ratio, 3)),\n", | ||
| " (\"Reaction equation\", rxn),\n", | ||
| " (\"E$_{rxt}$ per mol equation (kJ/mol)\", round(rxn_energy, 1)),\n", | ||
| " (\"E$_{rxt}$ per reactant atom (eV/atom)\", round(reactivity, 3)),\n", | ||
| " ]\n", | ||
| " )\n", | ||
| " for _, ratio, reactivity, rxn, rxn_energy in interface.get_kinks()\n", | ||
| "]\n", | ||
| "interface_reaction_table = pd.DataFrame(critical_rxns)\n", | ||
| "interface_reaction_table" | ||
| ] | ||
| }, |
There was a problem hiding this comment.
Optimize the data manipulation logic for generating the table of critical reactions.
- critical_rxns = [
- OrderedDict([
- ("Atomic fraction", round(ratio, 3)),
- ("Reaction equation", rxn),
- ("E$_{rxt}$ per mol equation (kJ/mol)", round(rxn_energy, 1)),
- ("E$_{rxt}$ per reactant atom (eV/atom)", round(reactivity, 3)),
- ])
- for _, ratio, reactivity, rxn, rxn_energy in interface.get_kinks()
- ]
+ critical_rxns = [
+ {
+ "Atomic fraction": round(ratio, 3),
+ "Reaction equation": rxn,
+ "E$_{rxt}$ per mol equation (kJ/mol)": round(rxn_energy, 1),
+ "E$_{rxt}$ per reactant atom (eV/atom)": round(reactivity, 3),
+ }
+ for _, ratio, reactivity, rxn, rxn_energy in interface.get_kinks()
+ ]Refactor the list comprehension for clarity and Pythonic style by using a dictionary directly instead of OrderedDict.
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.
| "from collections import OrderedDict\n", | |
| "\n", | |
| "import pandas as pd\n", | |
| "\n", | |
| "critical_rxns = [\n", | |
| " OrderedDict(\n", | |
| " [\n", | |
| " (\"Atomic fraction\", round(ratio, 3)),\n", | |
| " (\"Reaction equation\", rxn),\n", | |
| " (\"E$_{rxt}$ per mol equation (kJ/mol)\", round(rxn_energy, 1)),\n", | |
| " (\"E$_{rxt}$ per reactant atom (eV/atom)\", round(reactivity, 3)),\n", | |
| " ]\n", | |
| " )\n", | |
| " for _, ratio, reactivity, rxn, rxn_energy in interface.get_kinks()\n", | |
| "]\n", | |
| "interface_reaction_table = pd.DataFrame(critical_rxns)\n", | |
| "interface_reaction_table" | |
| ] | |
| }, | |
| "from collections import OrderedDict\n", | |
| "\n", | |
| "import pandas as pd\n", | |
| "\n", | |
| "critical_rxns = [\n", | |
| " {\n", | |
| " \"Atomic fraction\": round(ratio, 3),\n", | |
| " \"Reaction equation\": rxn,\n", | |
| " \"E$_{rxt}$ per mol equation (kJ/mol)\": round(rxn_energy, 1),\n", | |
| " \"E$_{rxt}$ per reactant atom (eV/atom)\": round(reactivity, 3),\n", | |
| " }\n", | |
| " for _, ratio, reactivity, rxn, rxn_energy in interface.get_kinks()\n", | |
| "]\n", | |
| "interface_reaction_table = pd.DataFrame(critical_rxns)\n", | |
| "interface_reaction_table" | |
| ] | |
| }, |
| "# Get interface reaction information for reactants LiCoO2 and Li3PS4 in open system to Co.\n", | ||
| "kinks_from_API = mpr.get_interface_reactions(\"LiCoO2\", \"Li3PS4\", open_el=\"Co\", relative_mu=-1, use_hull_energy=False)\n", | ||
| "\n", | ||
| "# Get information for the second critical reaction.\n", | ||
| "print(kinks_from_API[1])" | ||
| ] |
There was a problem hiding this comment.
Tip
Codebase Verification
The verification process has successfully identified the usage of the mpr.get_interface_reactions API call in the notebook file. However, the output from the script does not show any exception handling around this API call. This suggests that the initial review comment concerning the need to ensure that exception handling is implemented around these API calls is valid.
Given this information, it is appropriate to conclude that the review comment is correct and that the notebook should include exception handling to manage potential errors from the API call.
Ensure that the API calls for fetching interface reactions are correctly implemented and handle possible exceptions.
- The API call
mpr.get_interface_reactionsis used in the notebook without exception handling. It is recommended to wrap this call in a try-except block to handle potential errors gracefully.
Analysis chain
Ensure that the API calls for fetching interface reactions are correctly implemented and handle possible exceptions.
Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify that the API calls for fetching interface reactions handle exceptions.
# Test: Search for API calls and check for exception handling.
rg --type py 'mpr.get_interface_reactions' '2019-03-11-Interface Reactions.ipynb'
Length of output: 158
Script:
#!/bin/bash
# Find the correct path for the notebook file
fd '2019-03-11-Interface Reactions.ipynb'
# Assuming the correct path is found, re-run the rg command to check for API usage and exception handling
# Placeholder for the correct path, to be replaced after the first command's output
rg --type py 'mpr.get_interface_reactions' '<correct_path_to_notebook>'
Length of output: 228
Script:
#!/bin/bash
# Correctly use the found path to search for API usage and exception handling
rg --type py 'mpr.get_interface_reactions' 'examples/2019-03-11-Interface Reactions.ipynb'
Length of output: 223
|
@coderabbitai ignore Actions PerformedReviews paused. |
|
@janosh I think it is a great idea to bring this back to main pymatgen. |
cool, feel free to push the new lobster notebook to this PR and delete the old one |
|
@janosh. Can we set CodeRabbit to review only when we explicitly call it (On-Demand Reviews)? It's badly polluting conversations. What do you think? Lines 10 to 11 in a9da16e |
|
@janosh currently, the folders are not correctly linked. Some Notebooks require files. |
i suggested that to @shyuep a couple weeks ago but he was not in favor. i think it would require some community discussion |
|
Concur, I think this is a great idea! I’m not sure the genesis of matgenb, but I know that many people do not know it exists. I’ve been asked many times for example notebooks from people, and having some example notebooks directly in the pymatgen repo would definitely help with discoverability. |
|
The reason is because matgenb is not just about pymatgen. There are examples for pymatgen as well as non-pymatgen packages. Instead of unilaterally doing such stuff, I suggest you first ask and then do. It is a simple question or Issue. All this is leading to wasted effort. If you feel discoverability is an issue, by all means add a link to matgenb from pymatgen/examples. Also, matgenb is already mentioned multiple times in the docs. I can't help if people don't read the docs. |
|
I would like to clarify our intention, as proposed to @janosh. Our goal is to augment the |
|
@shyuep i'll address your points one by one.
i don't see that as being an issue.
like you said, there are multiple
finally, i would like push back on your last statement:
this is not true. you could simply not unilaterally close PRs to improve the docs, esp. if they attracted broad stakeholder support like this one! this too creates wasted effort by requiring me to argue what i believe are the obvious merits of this PR |
|
@janosh As I said, discuss with me. I don't want to go round and round with you in GitHub issues on this kind of thing. There are ways to improve the discoverability without doing this duplication. Note that matgenb is literally in the third paragraph in the intro of pymatgen.org. There is an entire section called Examples right after the Intro that talks about it. You are basically saying "oh damn I missed those obvious mentions and something must be wrong with discoverability." |
|
|
|
discoverability is only one of several problems. have you tried running the example notebooks? many of them are broken. by blocking this PR, you're preventing them from becoming better maintained and continuously tested. see again my OP:
|
|
By all means add CI to matgenb. Again, the answer cannot be "just move everything to the pymatgen repo to make life easier for the maintainer". By that argument, we should have one giant repo for all the codes. |
why not? that's exactly what should be done if it's the easiest way of improving usage examples we're talking about moving docs for pymatgen into the pymatgen repo... just like other packages have their example notebooks in the same repo.
that's basically what pymatgen is already! plus that's exactly how even huge companies like Google handle it internally: one giant monorepo. i really don't see how this is a counterargument |
|
My outside perspective: it would certainly be nice to ensure that these examples are still working in the future. Automatic execution in CI would be great. |
fix `core.operations` add types for site, mypy errors to fix remove no_type_check decorator move dunder methods to the top fix mypy errors remove debug tag improve `spectrum` finish `core.tensors` finish `xcfunc` fix hash of `SymmOp` finish `core.trajectory` Add types for `core.molecular_orbitals/operations/sites/spectrum/tensor/xcfunc` (materialsproject#3829) * fix `core.molecular_orbitals` * fix `core.operations` * add types for site, mypy errors to fix * remove no_type_check decorator * move dunder methods to the top * fix mypy errors * remove debug tag * improve `spectrum` * finish `core.tensors` * finish `xcfunc` * fix hash of `SymmOp` * add a missing type * Revert "fix hash of `SymmOp`" This reverts commit bf2a953. * fix some types in operations * "TEST": remove one unused var, relocate another * final tweak types * avoid hard-code class name * format tweaks * type tweak * fix unit test * replace all `[0:x]` with `[:x]` Guard `TYPE_CHECKING` only imports (materialsproject#3827) * flake8 --select=TYP001 * fix type import * format docstring to google style * fix more unguarded typecheck imports * format more docs to Google format * trigger CI (better var names) --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com> move structures out of util dir (materialsproject#3831) Add matgenb in the aux link section. Because apparently it is not "discoverable". Fixes materialsproject#3811. More updates. Move AIMS notebooks to matgenb. Add README in examples folder that point to proper resources. Improve type annotations and comments for `io.cif` (materialsproject#3820) * remove duplicate warning * type and docstring tweaks * add some types for io.cif * add comments and types * temp save of some formatting * revert functional change * revert unrelated changes * fix unit test * remove update that does nothing * relocate `sub_space_group` and `space_groups` to their usage * add more types * pre-commit auto-fixes * breaking: make parse_magmom/oxi_stats private * remove merge header * fix unit test * add more types * fix mypy errors * add a few spaces * remove if name ==main * simplify "check to see if" in comments * final tweaks * revise docstring * replace deprecated abc.abstractproperty * add missing doc strings and standardize existing * breaking: fix typo in method name orthongonalize_structure * revert accidental change in test_composition.py --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
* relocate dunder methods to top for core.trajectory fix `core.operations` add types for site, mypy errors to fix remove no_type_check decorator move dunder methods to the top fix mypy errors remove debug tag improve `spectrum` finish `core.tensors` finish `xcfunc` fix hash of `SymmOp` finish `core.trajectory` Add types for `core.molecular_orbitals/operations/sites/spectrum/tensor/xcfunc` (#3829) * fix `core.molecular_orbitals` * fix `core.operations` * add types for site, mypy errors to fix * remove no_type_check decorator * move dunder methods to the top * fix mypy errors * remove debug tag * improve `spectrum` * finish `core.tensors` * finish `xcfunc` * fix hash of `SymmOp` * add a missing type * Revert "fix hash of `SymmOp`" This reverts commit bf2a953. * fix some types in operations * "TEST": remove one unused var, relocate another * final tweak types * avoid hard-code class name * format tweaks * type tweak * fix unit test * replace all `[0:x]` with `[:x]` Guard `TYPE_CHECKING` only imports (#3827) * flake8 --select=TYP001 * fix type import * format docstring to google style * fix more unguarded typecheck imports * format more docs to Google format * trigger CI (better var names) --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com> move structures out of util dir (#3831) Add matgenb in the aux link section. Because apparently it is not "discoverable". Fixes #3811. More updates. Move AIMS notebooks to matgenb. Add README in examples folder that point to proper resources. Improve type annotations and comments for `io.cif` (#3820) * remove duplicate warning * type and docstring tweaks * add some types for io.cif * add comments and types * temp save of some formatting * revert functional change * revert unrelated changes * fix unit test * remove update that does nothing * relocate `sub_space_group` and `space_groups` to their usage * add more types * pre-commit auto-fixes * breaking: make parse_magmom/oxi_stats private * remove merge header * fix unit test * add more types * fix mypy errors * add a few spaces * remove if name ==main * simplify "check to see if" in comments * final tweaks * revise docstring * replace deprecated abc.abstractproperty * add missing doc strings and standardize existing * breaking: fix typo in method name orthongonalize_structure * revert accidental change in test_composition.py --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com> * remove accidental change * fix mypy errors * fix mypy error * add some for core.units * rename `ArrayWithFloatWithUnit` to `ArrayWithUnit` in comment * fix mypy errors * tweak docstring for units * nest internal funcs for units * simplify module docstring * revert to accessing private attrib * fix unit test and revert support for `Unit` * use `str` over `Unit` * support unit as both str and Unit * tweak docstring example * improve IndexError messages * test_index_error --------- Co-authored-by: Janosh Riebesell <janosh.riebesell@gmail.com>
"discoverable". Fixes materialsproject#3811.
motivation: current pymatgen docs are purely class/method and function doc strings. pretty dry stuff which mostly helps you if you already have a good idea of what you want to do and just need to be reminded of what parameters are available or what their defaults are. to get familiar with a new part of pymatgen, you often have to go straight to reading the source code which can be a significant learning curve (a high activation barrier if you like 😄), esp. for beginners.
luckily
matgenbhas provided a decent collection of usage example notebooks for years. given that i only discovered it over a year after starting to contribute to pymatgen (not the start of using it mind you), it's likely unknown to a large part of the pymatgen user base.merging those Jupyter notebooks into the
pymatgenrepo will greatly increase visibility. it also allows us to subject them topymatgenhigher code quality standards and potentially even integrate them into CI to ensure they don't break over time.To do
decide what format we want the examples to have. currently jupyter notebooks but could be converted to simple python scripts with VSCode style
# %%cell delimeters. these can be converted to.ipynbformat on the fly and from there converted to HTML to be rendered in thepymatgendocs at some point. this simplifies running the scripts in CI for testing but has the significant disadvantage that cell outputs cannot be savedpinging @DanielYang59 as we're planning to pool our effort on this PR. also pinging @JaGeo @mkhorton since it ties in with private communication about lowering the barrier to entry to the MP ecosystem