-
Notifications
You must be signed in to change notification settings - Fork 25
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
Develop #380
Conversation
Sync Develop Branch with Master branch
Drop setup.py
Add a Changelog to the Documentation
Fix the MP URL to ensure tests run
for more information, see https://pre-commit.ci
validated the approximate values in the docs with the new intermetallic scoring function
updates: - [github.com/astral-sh/ruff-pre-commit: v0.6.1 → v0.9.3](astral-sh/ruff-pre-commit@v0.6.1...v0.9.3) - [github.com/pre-commit/pre-commit-hooks: v4.6.0 → v5.0.0](pre-commit/pre-commit-hooks@v4.6.0...v5.0.0) - [github.com/igorshubovych/markdownlint-cli: v0.41.0 → v0.44.0](igorshubovych/markdownlint-cli@v0.41.0...v0.44.0) - [github.com/kynan/nbstripout: 0.7.1 → 0.8.1](kynan/nbstripout@0.7.1...0.8.1) - [github.com/codespell-project/codespell: v2.3.0 → v2.4.0](codespell-project/codespell@v2.3.0...v2.4.0) - [github.com/RobertCraigie/pyright-python: v1.1.376 → v1.1.392.post0](RobertCraigie/pyright-python@v1.1.376...v1.1.392.post0) - [github.com/adamchainz/blacken-docs: 1.18.0 → 1.19.1](adamchainz/blacken-docs@1.18.0...1.19.1)
for more information, see https://pre-commit.ci
[WIP] Update continuous integration and contributing guide
[pre-commit.ci] pre-commit autoupdate
metallicity - allowing us to be more general with the features added opening up the possibility of adding more features for enhanced metal detection in the future Includes addition of tests, docs, and tutorials changes
get_d_block_element_fraction
SMACT Metallicity Handling Enhancements
fix: correct function call for d block element count
Updated oxidation.py and its tests
…space_visualisation
WalkthroughThis pull request introduces a new GitHub Actions workflow that automatically updates the changelog on tag pushes, using Ruby and a Python filter script to remove unwanted bot entries. Additionally, the pre-commit configuration is extended with a new CI section and updated repository versions. The contributing guidelines have been expanded in both the root and documentation files, and the documentation itself is updated with new index entries, configuration adjustments, and illustrative examples. Finally, numerous JSON files have been added to the tutorials for detailed material property representations, and some notebooks have been enhanced with improved output displays. Changes
Sequence Diagram(s)sequenceDiagram
participant T as Tag Push
participant GH as GitHub Runner
participant RB as Ruby Setup
participant PG as Python Script
participant CM as Commit Module
T->>GH: Trigger workflow on tag push "v*"
GH->>GH: Checkout repository
GH->>RB: Set up Ruby (v3.3) and install github_changelog_generator
RB->>GH: Generate changelog (docs/CHANGELOG.md)
GH->>PG: Set up Python (v3.10) and upgrade pip
PG->>GH: Run filter_changelog.py to remove bot entries
GH->>CM: Execute pre-commit hooks, commit and push changelog updates
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
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 (
|
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: 1
🧹 Nitpick comments (37)
docs/tutorials/data/binary/mp_data/AcIn3.json (2)
1-10
: Metadata and Composition Fields Review.
The initial segment of the JSON file defines several metadata and composition keys. Many of these (e.g."builder_meta"
,"nsites"
,"elements"
,"composition"
) are currently set to null. Please ensure that these null values are intentional placeholders based on your schema and that users of this data are aware when entries are not available. The"formula_pretty"
and"formula_anonymous"
values clearly represent AcIn3 and its simplified composition, which is good.
22-100
: Structural and Site Information Assessment.
This section provides a detailed description of the material structure using the pymatgen schema. The lattice matrix, periodic boundary conditions, and derived lattice parameters (lines 26–40) appear to be correctly defined. The"sites"
array (lines 42–98) correctly lists each atomic position with both fractional ("abc"
) and absolute ("xyz"
) coordinates. One minor suggestion: the"magmom"
values are expressed as-0.0
—while this is not technically incorrect, standardising these to0.0
might improve clarity for future readers.docs/tutorials/data/binary/mp_data/AcAg.json (4)
1-7
: Metadata Fields: Review Optionality.
The keys such as"builder_meta"
,"nsites"
,"elements"
,"nelements"
,"composition"
, and"composition_reduced"
are all set tonull
. Please confirm that these values are intentionally left empty (i.e. to be populated later or optional) rather than omitted, as this might simplify the file if they are not used.
41-57
: Atomic Sites: Check Numerical Precision.
Both atomic sites for Ac and Ag are properly defined. However, the appearance of-0.0
and values like0.500001
in the"abc"
coordinates may be artefacts of floating-point precision. It might be beneficial to normalise these values (e.g. converting-0.0
to0.0
) for clarity and consistency.
75-106
: Additional Properties: Validate Null Assignments.
A considerable number of additional properties (e.g."bandstructure"
,"dos"
,"is_magnetic"
, etc.) are set tonull
. Confirm that these placeholders accurately represent either missing data or are intentionally left for future computation, so that the schema remains coherent across similar material data files.
107-163
: Fields Not Requested: Ensure Schema Clarity.
The"fields_not_requested"
array lists numerous keys that are omitted from the primary data output. This comprehensive list is useful for filtering; however, please verify that it remains updated and consistent with other JSON files in the dataset.docs/tutorials/data/binary/mp_data/AcSc3.json (7)
1-7
: Metadata Keys and Null Values
The opening keys (e.g."builder_meta"
,"nsites"
,"elements"
, etc.) are currently set to null. This is acceptable if these values are intentionally left blank; however, if available, populating them (for instance, automatically setting"nsites"
to the number of sites) could improve data completeness.
8-10
: Formula Consistency and Chemical System
The"formula_pretty"
and"formula_anonymous"
fields are correctly provided. In addition, consider populating the"chemsys"
field if the chemical system information is available—this could be beneficial for filtering or querying the dataset.
11-14
: Physical Properties and Symmetry Information
The physical properties ("volume"
,"density"
,"density_atomic"
) are well defined. The"symmetry"
field is currently null; if symmetry data can be derived or is available, including it would enhance the structural description of the material.
22-41
: Structure Block Conformance
This section correctly utilises the pymatgen structure schema with fields like"@module"
,"@class"
, and lattice parameters. The lattice information (including the matrix, lattice constants, and angles) is consistently defined. It might be useful, if feasible, to derive the"nsites"
value from the sites array and update the metadata accordingly.
42-99
: Site Data Integrity
Each entry in the"sites"
array includes the expected keys (such as"species"
,"abc"
,"properties"
,"label"
, and"xyz"
). Ensure that the total number of sites (which appears to be 9) is consistent with any metadata fields (e.g."nsites"
) that might eventually be populated.
101-148
: Material Energy Properties Verification
The numerical properties (e.g."energy_per_atom"
,"formation_energy_per_atom"
,"energy_above_hull"
, etc.) are clearly defined. Please verify that the values provided are accurate and consistent with your intended dataset. For example, a"band_gap"
of 0.0 may be correct, but it is worth confirming if this material is indeed metallic or if this is a placeholder value.
149-205
: Unrequested Fields Listing
The"fields_not_requested"
array comprehensively lists many fields that are omitted. This is useful for clarity. Just ensure that this list remains in sync with the overall data schema, especially if additional fields might be added to the project in future iterations.docs/tutorials/data/binary/mp_data/Ac3Tm.json (3)
1-7
: Metadata and Initial Keys.The initial keys and metadata placeholders (with many
null
values) are defined clearly. Ensure that thesenull
entries are intentional and will be updated as data becomes available.
8-10
: Material Identification.The fields
"formula_pretty": "Ac3Tm"
and"formula_anonymous": "AB3"
are correctly specified. Consider setting a value for"chemsys"
if applicable to maintain consistency across the dataset.
41-72
: Atomic Sites Definition.The
"sites"
array lists four atomic sites with appropriatespecies
, fractional (abc
) and Cartesian (xyz
) coordinates. Note that the"magmom"
value is given as-0.0
in several entries; for clarity, you might consider standardising this to0.0
unless the negative zero carries specific semantic meaning.docs/tutorials/data/binary/mp_data/AcCd3.json (2)
1-7
: Optional Metadata Fields HandlingThe keys such as
"builder_meta"
,"nsites"
,"elements"
,"nelements"
,"composition"
, and"composition_reduced"
are currently set tonull
. If these are not required for the present dataset, consider either removing them or adding a comment/documentation note to clarify their intended future use.
8-10
: Chemical Formula and System IdentificationThe
"formula_pretty"
and"formula_anonymous"
fields are well defined. However,"chemsys"
remainsnull
. If the chemical system information (e.g."Ac-Cd"
) is available or can be derived, populating this field could improve metadata completeness.docs/tutorials/data/binary/mp_data/Ac3Sn.json (4)
1-7
: Metadata Keys and Null Values
The keys (e.g."builder_meta"
,"nsites"
,"elements"
) are currently set tonull
. Please verify that thesenull
values are intentional placeholders. If these fields are not required, consider removing them or adding documentation to explain their current omission.
11-21
: Physical Properties and Material Identification
The provided numerical properties such as"volume"
,"density"
,"density_atomic"
, and"material_id": "mp-861939"
appear consistent with expected material data. For clarity and future maintainability, consider adding comments or documentation regarding the units for properties like density, if relevant in the wider context of the project.
22-72
: Structure Object Representation
The JSON segment for the"structure"
key conforms to thepymatgen
format, which is ideal for interoperability. A few points to consider:
- The lattice
"matrix"
contains values such as-0.0
which, although numerically equivalent to0.0
, could be normalised for clarity.- The
"sites"
array correctly defines atomic species and positions; the slight inconsistencies in floating point representations (e.g.-0.0
) are acceptable but may be cleaned up if you desire a more polished output.
Overall, the structure segment is well organised.
121-177
: Fields Not Requested Listing
The"fields_not_requested"
array is extensive and methodically excludes keys that are not central to the current data representation. For improved maintainability, you might consider generating or maintaining this list from a central configuration if similar JSON files are used across the project. This would help prevent the list from becoming outdated as the underlying data schema evolves.docs/tutorials/data/binary/mp_data/AcNb3.json (4)
1-7
: Consider Omitting Unused Null Fields
The top-level keys such as"builder_meta"
,"nsites"
,"elements"
, etc., are set tonull
. If these fields are not utilised downstream, consider omitting them to improve clarity and reduce file noise; otherwise, please clearly document their intended purpose.
11-13
: Check Numeric Property Precision
Properties like"volume"
,"density"
, and"density_atomic"
are specified with sufficient precision. Ensure this level of detail is aligned with the requirements for downstream applications.
73-120
: Examination of Material Property Fields
Energy-related fields such as"energy_per_atom"
,"formation_energy_per_atom"
, and"energy_above_hull"
appear clearly defined. Confirm that thenull
entries for other properties are intentional and consistent with the schema your project requires.
121-177
: Ensure 'fields_not_requested' is Up-to-Date
The"fields_not_requested"
array comprehensively lists keys that are intentionally omitted from processing. Please verify that this list remains current with the overall schema and that no critical fields are inadvertently excluded.dev_scripts/filter_changelog.py (1)
1-35
: Improve error handling and file I/O in the changelog filter script.The script functionality works, but there are several areas that could be improved:
- The file is opened twice (once for reading, once for writing) which could cause issues if the file is locked or if errors occur.
- The skipping logic is complex and might not handle all cases correctly.
- The script lacks error handling for file operations.
Consider implementing these improvements:
- def remove_github_actions_entries(changelog_path: str) -> None: + def remove_github_actions_entries(changelog_path: str) -> int: """Remove GitHub Actions bot entries from the changelog. Args: changelog_path: Path to the changelog file. + + Returns: + int: Number of entries removed. """ - with open(changelog_path) as file: - lines = file.readlines() + try: + with open(changelog_path) as file: + lines = file.readlines() - with open(changelog_path, "w") as file: - skip = False - for line in lines: - if re.search(r"\[github-actions\[bot\]\]", line): - skip = True - elif skip and line.strip() == "": - skip = False - elif not skip: - file.write(line) + filtered_lines = [] + skip = False + removed_count = 0 + + for line in lines: + if re.search(r"\[github-actions\[bot\]\]", line): + skip = True + removed_count += 1 + elif skip and line.strip() == "": + skip = False + elif not skip: + filtered_lines.append(line) + + with open(changelog_path, "w") as file: + file.writelines(filtered_lines) + + return removed_count + except IOError as e: + print(f"Error processing changelog file: {e}") + return 0In the main block:
if __name__ == "__main__": parser = argparse.ArgumentParser(description="Remove GitHub Actions bot entries from changelog.") parser.add_argument("changelog_path", type=str, help="Path to the changelog file") args = parser.parse_args() - remove_github_actions_entries(args.changelog_path) + try: + removed = remove_github_actions_entries(args.changelog_path) + print(f"Successfully removed {removed} GitHub Actions bot entries from the changelog.") + except Exception as e: + print(f"An error occurred: {e}") + exit(1)docs/CONTRIBUTING.md (2)
60-62
: Unit Test Execution InstructionThe guidance for running unit tests is straightforward. Consider rephrasing to remove the word “of” for conciseness—for example, “ensure that all unit tests pass on your own machine by running
pytest -v
.”🧰 Tools
🪛 LanguageTool
[style] ~61-~61: Consider removing “of” to be more concise
Context: ...are finished with the work, ensure that all of the unit tests pass on your own machine by ...(ALL_OF_THE)
78-79
: Merge Conflict WarningThe reminder regarding ‘git pull’ is useful. Consider rephrasing it for brevity (e.g. “Note: ‘git pull’ merges fetched changes, which can cause conflicts if your master branch differs from upstream”).
🧰 Tools
🪛 LanguageTool
[style] ~78-~78: Consider shortening or rephrasing this to strengthen your wording.
Context: ...uld lead to merge conflicts if you have made changes to the master branch. ## Pull requests F...(MAKE_CHANGES)
CONTRIBUTING.md (1)
1-118
: Unified Contribution GuidelinesThe CONTRIBUTING.md document offers clear and detailed instructions for contributors. To streamline maintenance, ensure consistency with the version in the docs folder – consider linking between the two rather than maintaining duplicate documents.
🧰 Tools
🪛 LanguageTool
[style] ~10-~10: Would you like to use the Oxford spelling “summarized”? The spelling ‘summarised’ is also correct.
Context: ...he steps for a new piece of work can be summarised as follows: 1. Push up or create [an i...(OXFORD_SPELLING_Z_NOT_S)
[style] ~61-~61: Consider removing “of” to be more concise
Context: ...are finished with the work, ensure that all of the unit tests pass on your own machine by ...(ALL_OF_THE)
[style] ~78-~78: Consider shortening or rephrasing this to strengthen your wording.
Context: ...uld lead to merge conflicts if you have made changes to the master branch. ## Pull requests F...(MAKE_CHANGES)
docs/tutorials/crystal_space.ipynb (7)
97-105
: Add a summarising message after the generation steps.While these progress statements are clear, consider ending with a summarising message, e.g., "Successfully generated X combinations in Y seconds", to reassure the user that the steps have completed.
107-129
: Use a consistent progress bar approach.The progress bars appear in the stderr stream, which could be overshadowed by other logs. Consider either using a single consistent logging method or ensuring these logs are distinctly visible.
147-147
: Document the 'smact14' parameter.Include explanations about how
'smact14'
differs from other sets and why it's used. This helps ensure clarity when reading the code or tutorial.
153-245
: Consider truncating extensive table outputs.Displaying 225,879 rows can overwhelm the notebook. Use a summarised output (for example,
df_smact.head()
) or a sample to improve readability and performance.
284-284
: Encourage secure handling of API keys.Storing API keys in environment variables or a credentials management system is preferable to embedding an empty string placeholder, especially for public-facing notebooks.
300-488
: Reduce verbosity for better readability.While detailed download logs are helpful, providing a more compact summary (or an option to enable/disable verbose mode) can enhance the user experience in a tutorial.
1022-1044
: Vectorised label creation.Your current apply-based mapping works, but consider a direct vectorised approach for large data sets. For example, using
df_smact['smact_allowed'] * 2 + df_mp['mp']
as a key for a dictionary could be faster and more readable.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (9)
docs/tutorials/data/binary/df_binary_category.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/df_binary_label.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/df_binary_sample.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/embeddings/embeddings_magpie.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/embeddings/embeddings_mat2vec.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/embeddings/embeddings_megnet16.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/embeddings/embeddings_oliynyk.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/embeddings/embeddings_random_200.pkl
is excluded by!**/*.pkl
docs/tutorials/data/binary/embeddings/embeddings_skipatom.pkl
is excluded by!**/*.pkl
📒 Files selected for processing (103)
.github/workflows/update-changelog.yml
(1 hunks).pre-commit-config.yaml
(3 hunks)CONTRIBUTING.md
(3 hunks)dev_scripts/filter_changelog.py
(1 hunks)docs/CHANGELOG.md
(1 hunks)docs/CONTRIBUTING.md
(1 hunks)docs/conf.py
(3 hunks)docs/examples.rst
(1 hunks)docs/examples/metallicity.ipynb
(1 hunks)docs/index.rst
(1 hunks)docs/smact.metallicity.rst
(1 hunks)docs/smact.rst
(1 hunks)docs/tutorials.rst
(1 hunks)docs/tutorials/crystal_space.ipynb
(5 hunks)docs/tutorials/crystal_space_visualisation.ipynb
(2 hunks)docs/tutorials/data/binary/mp_data/Ac2Mg.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac2O3.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac2S3.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Ag.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Au.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Br.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Cd.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Ce.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Cl.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Cr.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Dy.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Er.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Eu.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Hg.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Ho.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3I.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3In.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3La.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Lu.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Mo.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Nb.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Nd.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Np.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Pa.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Pm.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Pr.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Sc.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Si.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Sm.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Sn.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Tb.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Th.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Ti.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Tl.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Tm.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3V.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Y.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Yb.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Zn.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ac3Zr.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcAg.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcAg3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcAl3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcAu3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcBr3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcCd3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcCe3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcCl3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcCu3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcDy3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcEr3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcEu3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcF3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcGa3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcGe3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcH2.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcH3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcHg3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcI3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcIn3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcLa.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcLa3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcMg.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcMg3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcMg5.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcN.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcNb3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcNd3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcO.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcPm3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcPr3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcSc3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcSe3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcSi3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcSm3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcTb3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcTe3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcTh.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcTh3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcY3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcYb3.json
(1 hunks)docs/tutorials/data/binary/mp_data/AcZn3.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ag2Br3.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ag2Cl3.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ag2F.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ag2F3.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ag2F5.json
(1 hunks)docs/tutorials/data/binary/mp_data/Ag2Hg.json
(1 hunks)
✅ Files skipped from review due to trivial changes (70)
- docs/smact.metallicity.rst
- docs/examples.rst
- docs/tutorials/data/binary/mp_data/Ac3Pr.json
- docs/tutorials/data/binary/mp_data/AcO.json
- docs/tutorials/data/binary/mp_data/AcEr3.json
- docs/tutorials/data/binary/mp_data/Ac3Cr.json
- docs/tutorials/data/binary/mp_data/AcCl3.json
- docs/tutorials/data/binary/mp_data/Ac3Br.json
- docs/tutorials/data/binary/mp_data/Ac3Nb.json
- docs/tutorials/data/binary/mp_data/AcCu3.json
- docs/tutorials/data/binary/mp_data/Ac3Tl.json
- docs/tutorials/data/binary/mp_data/AcZn3.json
- docs/tutorials/data/binary/mp_data/Ac3Zn.json
- docs/tutorials/data/binary/mp_data/AcMg5.json
- docs/tutorials/data/binary/mp_data/Ac3Si.json
- docs/tutorials/data/binary/mp_data/AcTb3.json
- docs/tutorials/data/binary/mp_data/Ac3Ho.json
- docs/tutorials/data/binary/mp_data/AcAg3.json
- docs/tutorials/data/binary/mp_data/AcLa.json
- docs/tutorials/data/binary/mp_data/AcPr3.json
- docs/tutorials/data/binary/mp_data/AcYb3.json
- docs/tutorials/data/binary/mp_data/AcDy3.json
- docs/tutorials/data/binary/mp_data/AcAl3.json
- docs/tutorials/data/binary/mp_data/AcH2.json
- docs/tutorials/data/binary/mp_data/Ac3Nd.json
- docs/tutorials/data/binary/mp_data/Ac3Hg.json
- docs/tutorials/data/binary/mp_data/Ac3V.json
- docs/tutorials/data/binary/mp_data/Ag2F5.json
- docs/tutorials/data/binary/mp_data/AcNd3.json
- docs/tutorials/data/binary/mp_data/Ac3Yb.json
- docs/tutorials/data/binary/mp_data/Ac3Tb.json
- docs/tutorials/data/binary/mp_data/AcSi3.json
- docs/tutorials/data/binary/mp_data/Ag2F3.json
- docs/tutorials/data/binary/mp_data/AcGe3.json
- docs/tutorials/data/binary/mp_data/AcF3.json
- docs/tutorials/data/binary/mp_data/AcTe3.json
- docs/tutorials/data/binary/mp_data/Ag2Hg.json
- docs/tutorials/data/binary/mp_data/Ag2Cl3.json
- docs/tutorials/data/binary/mp_data/Ag2Br3.json
- docs/tutorials/data/binary/mp_data/AcMg.json
- docs/tutorials/data/binary/mp_data/Ac3La.json
- docs/tutorials/data/binary/mp_data/AcN.json
- docs/tutorials/data/binary/mp_data/Ac3Au.json
- docs/tutorials/data/binary/mp_data/AcAu3.json
- docs/tutorials/data/binary/mp_data/Ac3Th.json
- docs/tutorials/data/binary/mp_data/Ac3Zr.json
- docs/tutorials/data/binary/mp_data/Ac3Sm.json
- docs/tutorials/data/binary/mp_data/AcEu3.json
- docs/tutorials/data/binary/mp_data/Ac3Mo.json
- docs/tutorials/data/binary/mp_data/AcBr3.json
- docs/tutorials/data/binary/mp_data/AcH3.json
- docs/tutorials/data/binary/mp_data/Ac3Pm.json
- docs/tutorials/data/binary/mp_data/Ac3Y.json
- docs/tutorials/data/binary/mp_data/AcPm3.json
- docs/tutorials/data/binary/mp_data/Ac3Ce.json
- docs/tutorials/data/binary/mp_data/AcLa3.json
- docs/tutorials/data/binary/mp_data/Ac3Cl.json
- docs/tutorials/data/binary/mp_data/AcGa3.json
- docs/tutorials/data/binary/mp_data/AcI3.json
- docs/tutorials/data/binary/mp_data/AcHg3.json
- docs/tutorials/data/binary/mp_data/Ac3Sc.json
- docs/tutorials/crystal_space_visualisation.ipynb
- docs/tutorials/data/binary/mp_data/Ac3Dy.json
- docs/tutorials/data/binary/mp_data/AcY3.json
- docs/tutorials/data/binary/mp_data/Ac3I.json
- docs/tutorials/data/binary/mp_data/Ag2F.json
- docs/tutorials/data/binary/mp_data/AcCe3.json
- docs/tutorials/data/binary/mp_data/AcMg3.json
- docs/tutorials/data/binary/mp_data/Ac3Lu.json
- docs/tutorials/data/binary/mp_data/Ac2S3.json
🧰 Additional context used
🪛 LanguageTool
docs/CONTRIBUTING.md
[style] ~10-~10: Would you like to use the Oxford spelling “summarized”? The spelling ‘summarised’ is also correct.
Context: ...he steps for a new piece of work can be summarised as follows: 1. Push up or create [an i...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~61-~61: Consider removing “of” to be more concise
Context: ...are finished with the work, ensure that all of the unit tests pass on your own machine by ...
(ALL_OF_THE)
[style] ~78-~78: Consider shortening or rephrasing this to strengthen your wording.
Context: ...uld lead to merge conflicts if you have made changes to the master branch. ## Pull requests F...
(MAKE_CHANGES)
CONTRIBUTING.md
[style] ~61-~61: Consider removing “of” to be more concise
Context: ...are finished with the work, ensure that all of the unit tests pass on your own machine by ...
(ALL_OF_THE)
[style] ~78-~78: Consider shortening or rephrasing this to strengthen your wording.
Context: ...uld lead to merge conflicts if you have made changes to the master branch. ## Pull requests F...
(MAKE_CHANGES)
docs/CHANGELOG.md
[misspelling] ~100-~100: This expression is normally spelled as one or with a hyphen.
Context: ...:** - ElementCountsParallel example is non deterministic [#268](https://github.com/WMD-group/SM...
(EN_COMPOUNDS_NON_DETERMINISTIC)
[grammar] ~109-~109: There seems to be a noun/verb agreement error. Did you mean “develops” or “developed”?
Context: ...62) Merged pull requests: - Merge develop branch changes into the master branch [...
(SINGULAR_NOUN_VERB_AGREEMENT)
[typographical] ~230-~230: It appears that a comma is missing.
Context: ...detect invalid covalent compounds? #58 - is it possible to use SMACT to detect vali...
(COMMA_BEFORE_QUESTION_WITH_MD)
[style] ~294-~294: Would you like to use the Oxford spelling “Organize”? The spelling ‘Organise’ is also correct.
Context: ...github.com//issues/35) - Organise examples and workflows [#17](https://g...
(OXFORD_SPELLING_Z_NOT_S)
[typographical] ~311-~311: It appears that a comma is missing.
Context: ... is no longer backwards compatible #42 - Do you have plan for more advanced Pauling...
(COMMA_BEFORE_QUESTION_WITH_MD)
[grammar] ~312-~312: There is an agreement error between ‘have’ and ‘plan’. Insert ‘a(n)’ or change the noun to plural.
Context: ...com//issues/42) - Do you have plan for more advanced Pauling rules for scr...
(PRP_VB_NN)
[locale-violation] ~337-~337: license must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...tObi](https://github.com/AntObi)) - New license [#24](https://github.com/WMD-group/SMA...
(LICENCE_LICENSE_NOUN_SINGULAR)
[style] ~363-~363: Would you like to use the Oxford spelling “polarizability”? The spelling ‘polarisability’ is also correct.
Context: .../issues/15) - Add atomic polarisability [#11](https://github.com/WMD-group/SMA...
(OXFORD_SPELLING_Z_NOT_S)
[style] ~374-~374: Would you like to use the Oxford spelling “polarizability”? The spelling ‘polarisability’ is also correct.
Context: ...o](https://github.com/keeeto)) - 11 add polarisability [#12](https://github.com/WMD-group/SMA...
(OXFORD_SPELLING_Z_NOT_S)
🔇 Additional comments (73)
docs/tutorials/data/binary/mp_data/AcIn3.json (1)
101-207
: Material Properties and Field Exclusion List Verification.
The final segment (lines 101–207) details various energy and stability properties. The key energy values (such as"energy_per_atom"
and"formation_energy_per_atom"
) and the stability flag ("is_stable": true
) are provided, which is essential for downstream analysis. The extensive"fields_not_requested"
list is clearly included to filter out non-critical fields. It is important to confirm that leaving many keys as null is consistent with your overall schema and data consumption expectations. If these nulls are expected, this section is well-prepared.docs/tutorials/data/binary/mp_data/AcAg.json (4)
8-17
: Material Identification: Verify Completeness.
The fields"formula_pretty"
,"formula_anonymous"
, and"material_id"
are correctly provided. However, notice that"composition"
and"chemsys"
arenull
. Ensure that this level of detail suffices for downstream applications or if more specific compositional data should be included.
22-40
: Lattice Structure: Validate Crystallographic Consistency.
The lattice block, including the 3×3 matrix, periodic boundary conditions ("pbc"
), and derived parameters ("a"
,"b"
,"c"
,"alpha"
,"beta"
,"gamma"
, and"volume"
), appears well-structured. It is recommended to verify that the provided"volume"
is consistent with the lattice parameters to ensure that the crystallographic data is accurate.
59-74
: Energy and Electronic Properties: Confirm Accuracy and Units.
The energy-related fields (e.g."energy_per_atom"
,"formation_energy_per_atom"
,"energy_above_hull"
) along with electronic properties (such as"band_gap"
and"efermi"
) appear correctly specified. Please ensure that these numerical values are in the expected units and that they align with the source data.
1-165
: Overall File Structure: Confirm Schema Conformance.
The new JSON file follows a detailed and consistent schema for representing material properties, lattice data, and atomic sites. Ensure that the file aligns with the project's data standard and that all mandatory fields (with accurate data or intentionalnull
assignments) are provided.docs/tutorials/data/binary/mp_data/AcSc3.json (1)
206-207
: JSON Formatting and File Closure
The file concludes with a closing brace and appears well-formatted overall. There are no formatting issues detected.docs/tutorials/data/binary/mp_data/Ac3Tm.json (7)
11-21
: Core Material Properties.The material’s core properties—such as
volume
,density
, anddensity_atomic
—along with the uniquematerial_id
are well detailed. It would be beneficial to double-check that these numerical values match expected physical data.
22-40
: Structure and Lattice Details.The
"structure"
section is well defined using thepymatgen.core.structure
class. The lattice information, including the matrix, periodic boundary conditions, cell parameters, angles, and derived volume, is accurate. Minor rounding differences (e.g. between the overallvolume
and the lattice’svolume
) are acceptable.
73-90
: Energy and Stability Metrics.The additional fields reporting energy metrics (
energy_per_atom
,formation_energy_per_atom
,energy_above_hull
) and the stability flag (is_stable
) are clearly provided. It is recommended to verify that these computed values correspond to the expected database values.
91-100
: Electronic and Magnetic Properties.Fields such as
"band_gap"
,"dos"
, and"total_magnetization"
are present and correctly formatted. Since several electronic and magnetic properties are set tonull
, please confirm that this is deliberate and that missing data will be handled appropriately downstream.
101-120
: Additional Physical Properties.The remaining physical properties (e.g. moduli, Poisson ratio, additional energy components) are included as per the schema. Leaving them as
null
is acceptable if no data is available; just ensure consistency with other similar files.
121-177
: Fields Not Requested List.The
"fields_not_requested"
array comprehensively lists keys that are to be omitted from output. This exhaustive list appears consistent with similar datasets—just ensure it is kept up-to-date with any schema revisions.
178-179
: File Termination.The JSON object is properly closed. No issues detected at the end of the file.
docs/tutorials/data/binary/mp_data/AcCd3.json (5)
11-13
: Physical Properties VerificationThe values for
"volume"
,"density"
, and"density_atomic"
appear correctly formatted and plausible. No issues detected here.
22-40
: Structure Definition and Lattice ParametersThe
"structure"
object adheres to the pymatgen JSON format. The lattice information—including the matrix, cell parameters ("a"
,"b"
,"c"
,"alpha"
,"beta"
,"gamma"
), volume, and periodic boundary conditions—is clearly and correctly specified.
42-99
: Atomic Sites Data ConsistencyThe
"sites"
array details the positions, species, and properties for each atomic site. With 2 Ac sites and 6 Cd sites, the overall composition reduces correctly to the provided formula"AcCd3"
. Please verify that the site labelling and occupancy details are intentional and well-documented.
100-112
: Material Energetic PropertiesThe energetic properties such as
"energy_per_atom"
,"formation_energy_per_atom"
,"band_gap"
, and"efermi"
are present and appear to be correctly formatted. These values are consistent with typical outputs for materials data.
149-205
: Excluded Fields ListThe
"fields_not_requested"
array comprehensively lists the keys that have been intentionally omitted from the output. This approach clearly delineates included versus excluded data. Ensure that this list is kept up-to-date if the dataset schema evolves.docs/tutorials/data/binary/mp_data/Ac3Sn.json (2)
8-10
: Formula Information Consistency
The fields"formula_pretty": "Ac3Sn"
and"formula_anonymous": "AB3"
look appropriate; however, note that"chemsys"
is alsonull
. Please confirm that this is intended and that no chemical system information should be derived from the provided composition data.
73-90
: Energy and Stability Properties
The energy-related keys (e.g."energy_per_atom"
,"formation_energy_per_atom"
,"energy_above_hull"
) and stability indicators like"is_stable": true
are clearly defined. The numerical values appear reasonable. No issues are apparent in this section.docs/tutorials/data/binary/mp_data/AcNb3.json (3)
8-10
: Validation of Formula Attributes
The"formula_pretty"
and"formula_anonymous"
fields are defined in accordance with standard conventions. Their current values appear correct.
26-40
: Verify Lattice Consistency
The"lattice"
object provides both a matrix representation and derived values ("a"
,"b"
,"c"
,"alpha"
,"beta"
,"gamma"
, and"volume"
). Please verify that these values are self-consistent and accurately represent the intended crystal structure.
42-70
: Review of Atomic Site Configurations
The"sites"
array correctly lists the atomic sites with detailed fields including"species"
, fractional coordinates ("abc"
), magnetic moment ("magmom"
), and Cartesian coordinates ("xyz"
). Double-check that the occupancy and magnetic properties are accurate and that the positions are correctly determined by your data source.docs/tutorials/data/binary/mp_data/Ac3Ti.json (10)
1-9
: Schema Metadata Fields ReviewThe metadata fields at the start are well-organised. Note that several keys (e.g.
"builder_meta"
,"nsites"
,"elements"
, etc.) are set tonull
. Please confirm that these are intended as placeholders until actual data is available.
10-17
: Material Basic Properties CheckThe fields
"formula_pretty"
and"formula_anonymous"
alongside"volume"
,"density"
, and"density_atomic"
provide a clear snapshot of the material’s properties. Additionally,"material_id": "mp-1183108"
appears to correctly reference the material.
18-21
: Deprecation and Update Fields ValidationThe deprecation-related fields (e.g.
"deprecated"
,"deprecation_reasons"
,"last_updated"
,"origins"
,"warnings"
) are all null. Please verify that this is intentional and that these fields will remain null or be updated later on if needed.
22-40
: Structure & Lattice Information AnalysisThe
"structure"
block is detailed, including the lattice matrix, periodic boundary conditions ("pbc"
), and geometric parameters (a, b, c, alpha, beta, gamma, volume). The negative values in the lattice matrix may initially appear unconventional but can be valid depending on the chosen coordinate system. It would be prudent to double-check these values against your domain-specific expectations.
41-41
: Empty Properties FieldThe empty
"properties"
dictionary within the structure is acceptable if no additional descriptive properties are required. Otherwise, consider populating it with any relevant structural attributes.
42-71
: Site Information CompletenessThe
"sites"
array clearly lists four sites with detailed entries for species, fractional coordinates ("abc"
), Cartesian coordinates ("xyz"
), and magnetic moment data ("magmom"
). Please verify that the magnitudes and signs of the magnetic moments for Ac and Ti are correct as per your experimental or computational data.
73-93
: Energetic Properties EvaluationThe energetic properties, including
"energy_per_atom"
,"formation_energy_per_atom"
, and"energy_above_hull"
, are specified. Notably, the equality of"formation_energy_per_atom"
and"energy_above_hull"
suggests a material on (or above) the stability threshold (as indicated by"is_stable": false
). It is recommended to verify that this equivalence is expected based on your energy calculations.
94-120
: Magnetic, Electronic & Mechanical Properties ReviewA variety of properties related to magnetism (e.g.
"is_magnetic"
,"total_magnetization"
), electronic structures ("band_gap"
,"efermi"
), and mechanical properties (e.g."bulk_modulus"
,"shear_modulus"
) are present. Although many of these fields arenull
, the provided values (like"total_magnetization": 0.070826
and"efermi": 6.28638329
) should be checked to ensure they meet expected quality and that consuming applications appropriately handle anynull
values.
121-177
: Fields Not Requested Consistency CheckThe
"fields_not_requested"
array lists numerous keys that are intentionally excluded from downstream processing. Ensure that this list remains up-to-date with any schema changes and that it does not inadvertently filter out any critical fields needed in future analyses.
178-178
: File Closure VerificationThe file is properly closed with the final brace. No issues detected here.
docs/tutorials.rst (1)
13-13
: Well structured addition of the new metallicity tutorial.The new tutorial "metallicity_classification" has been properly added to the toctree list, maintaining consistent formatting with the existing entries.
docs/smact.rst (1)
32-32
: LGTM: New metallicity submodule properly added.The addition of the "smact.metallicity" submodule to the documentation structure is well-formatted and aligns with the new tutorial that was added to tutorials.rst.
docs/index.rst (1)
59-60
: Good addition of documentation navigation links.Adding CHANGELOG and CONTRIBUTING to the documentation toctree will help users find important project information more easily.
docs/tutorials/data/binary/mp_data/Ac2O3.json (1)
1-186
: JSON Schema Conformity and Data IntegrityThis new JSON file is well structured and complies with the standard schema used for material data. The use of
null
for optional fields and the comprehensive set of keys are consistent with other files in the dataset. Please ensure that any fields for which data becomes available in the future are updated accordingly.docs/tutorials/data/binary/mp_data/Ac3Np.json (1)
1-207
: Overall Comment: JSON Structure and ConsistencyThe file follows the expected JSON format for material property entries. All keys—including the nested
structure
object and thefields_not_requested
array—are clearly defined, and the data types appear correct. This consistency should facilitate integration with downstream data processing workflows.docs/tutorials/data/binary/mp_data/AcTh3.json (1)
1-207
: JSON File Conformity and Data CompletenessThis file is a well-formed JSON document that mirrors the schema of other material data files. The structure information (lattice, sites, etc.) is detailed appropriately, and the use of
null
for unavailable data is consistent across entries.docs/CHANGELOG.md (1)
1-401
: Comprehensive Changelog DocumentationThe changelog is detailed and chronologically well organised, providing a clear history of updates, including bug fixes, enhancements, merged pull requests, and closed issues. A few minor grammatical and punctuation adjustments (as pointed out by static analysis hints) could further improve clarity, but overall the document meets the project’s documentation standards.
🧰 Tools
🪛 LanguageTool
[misspelling] ~100-~100: This expression is normally spelled as one or with a hyphen.
Context: ...:** - ElementCountsParallel example is non deterministic [#268](https://github.com/WMD-group/SM...(EN_COMPOUNDS_NON_DETERMINISTIC)
[grammar] ~109-~109: There seems to be a noun/verb agreement error. Did you mean “develops” or “developed”?
Context: ...62) Merged pull requests: - Merge develop branch changes into the master branch [...(SINGULAR_NOUN_VERB_AGREEMENT)
[typographical] ~230-~230: It appears that a comma is missing.
Context: ...detect invalid covalent compounds? #58 - is it possible to use SMACT to detect vali...(COMMA_BEFORE_QUESTION_WITH_MD)
[style] ~294-~294: Would you like to use the Oxford spelling “Organize”? The spelling ‘Organise’ is also correct.
Context: ...github.com//issues/35) - Organise examples and workflows [#17](https://g...(OXFORD_SPELLING_Z_NOT_S)
[typographical] ~311-~311: It appears that a comma is missing.
Context: ... is no longer backwards compatible #42 - Do you have plan for more advanced Pauling...(COMMA_BEFORE_QUESTION_WITH_MD)
[grammar] ~312-~312: There is an agreement error between ‘have’ and ‘plan’. Insert ‘a(n)’ or change the noun to plural.
Context: ...com//issues/42) - Do you have plan for more advanced Pauling rules for scr...(PRP_VB_NN)
[locale-violation] ~337-~337: license must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...tObi](https://github.com/AntObi)) - New license [#24](https://github.com/WMD-group/SMA...(LICENCE_LICENSE_NOUN_SINGULAR)
[style] ~363-~363: Would you like to use the Oxford spelling “polarizability”? The spelling ‘polarisability’ is also correct.
Context: .../issues/15) - Add atomic polarisability [#11](https://github.com/WMD-group/SMA...(OXFORD_SPELLING_Z_NOT_S)
[style] ~374-~374: Would you like to use the Oxford spelling “polarizability”? The spelling ‘polarisability’ is also correct.
Context: ...o](https://github.com/keeeto)) - 11 add polarisability [#12](https://github.com/WMD-group/SMA...(OXFORD_SPELLING_Z_NOT_S)
docs/tutorials/data/binary/mp_data/AcSm3.json (1)
1-207
: Correct JSON Data Format and ConsistencyThis new JSON file adheres to the standard format used across the database. The structure, key names, and value types (including the use of
null
for non-applicable fields) are consistent with other material files, ensuring uniformity within the dataset.docs/tutorials/data/binary/mp_data/Ac3Cd.json (1)
1-207
: New Material Data for Ac3Cd is well-structured and complete.
The JSON file accurately captures the crystal lattice, atomic sites, and physical properties consistent with pymatgen’s schema. The inclusion of a comprehensive"fields_not_requested"
list helps to clearly delineate non-essential data.docs/tutorials/data/binary/mp_data/AcSe3.json (1)
1-207
: AcSe3 JSON file is implemented correctly.
All necessary structural data—including lattice parameters, periodic boundary conditions and site-specific properties—are clearly defined. The file adheres to the project’s data schema and is easily extendable for further analysis.docs/tutorials/data/binary/mp_data/Ac2Mg.json (1)
1-193
: Ac2Mg JSON file is comprehensive and precise.
The material data, including detailed lattice descriptions and atomic site arrays, is well-defined. The use of the"fields_not_requested"
array ensures that only the relevant properties are focussed on, thereby enhancing clarity and maintainability.docs/tutorials/data/binary/mp_data/AcTh.json (1)
1-165
: AcTh JSON file has been formatted accurately.
The file clearly delineates the lattice parameters, atomic sites and associated energy properties. The structure is consistent with the established conventions in the repository, making it a reliable addition to the materials dataset.docs/tutorials/data/binary/mp_data/Ac3In.json (1)
1-207
: Ac3In JSON file provides detailed and accurate material data.
The crystal structure information—including lattice matrix, periodic boundary conditions and site-specific properties—is effectively captured. The overall structure is consistent and facilitates both computational materials science applications and future data integrations.docs/tutorials/data/binary/mp_data/Ac3Ag.json (1)
1-179
: JSON Schema Conformance and Data IntegrityThe JSON file is well organised and adheres to the expected schema for material data. Please verify that the null entries (e.g. "builder_meta", "nsites", etc.) are intentional placeholders within your data pipeline.
docs/tutorials/data/binary/mp_data/Ac3Eu.json (1)
1-207
: Comprehensive Material Data RepresentationThe file accurately encapsulates detailed structural and energetic properties for the material Ac3Eu. The schema is consistent throughout; please ensure that the null fields are suitably handled downstream.
docs/CONTRIBUTING.md (14)
1-4
: Introduction ClarityThe introductory section clearly explains the purpose of the document. You might consider adding a brief note regarding the contribution standards to set expectations early on.
5-9
: Workflow Section IntroductionThe opening of the workflow section succinctly outlines the process. No issues detected here.
10-20
: Detailed Contribution Workflow (Steps 1–3)The initial steps—including issue creation, forking, and cloning—are well described with clear command examples.
🧰 Tools
🪛 LanguageTool
[style] ~10-~10: Would you like to use the Oxford spelling “summarized”? The spelling ‘summarised’ is also correct.
Context: ...he steps for a new piece of work can be summarised as follows: 1. Push up or create [an i...(OXFORD_SPELLING_Z_NOT_S)
21-26
: UV Package Manager InstallationThe instructions for installing the uv package manager are concise and clear. Ensure that users have appropriate permissions when installing new packages.
27-35
: Virtual Environment SetupThe steps to create and activate a virtual environment using uv (with an alternative note for mamba) are effective and user-friendly.
38-44
: Editable Mode Installation InstructionsThe commands for installing the package in editable mode, along with the installation of pre-commit hooks, are well presented. It is advisable to periodically review the dependency list ([dev, docs, optional]) for any updates.
45-50
: Branch Creation GuidelinesThe instructions for creating a new branch from master are clear. A gentle reminder to update master prior to branching could further help avoid conflicts.
51-59
: Commit and Push ProceduresThe directions for staging changes, committing, and pushing your branch are succinct and adhere to best practice.
63-67
: Opening Pull Request GuidanceThe pull request instructions, including the remote configuration command, are clear and include useful links for further information.
68-77
: Upstream Repository ConfigurationThe commands provided for adding an upstream remote and pulling the latest changes are well documented and should help maintain synchronisation with the master branch.
80-90
: Pull Requests OverviewThe section on pull requests is informative, with clear pointers on title composition and issue referencing. The recommended reading link is a valuable addition.
91-97
: Development RequirementsThe instructions for installing local development packages and setting up the environment are comprehensive and easy to follow.
98-109
: Pre-commit Hooks and TestingThis section effectively explains how to install and utilise pre-commit hooks. It might be beneficial to occasionally remind users that running all hooks is optional.
110-118
: Documentation Rendering InstructionsThe commands for rendering the documentation with Sphinx are precise. This will help ensure that documentation is both complete and correctly formatted.
docs/tutorials/data/binary/mp_data/Ac3Pa.json (1)
1-207
: Robust Material Data Representation for Ac3PaThis JSON file provides a thorough set of data for the material Ac3Pa. The structure and format are consistent with your other material files. Please verify that the null fields and placeholder values are an intentional part of your data model.
.github/workflows/update-changelog.yml (1)
1-59
: Well-structured workflow for automatic changelog updates.This new GitHub Actions workflow is triggered on tag pushes and handles the changelog generation and update process efficiently. The use of both Ruby for changelog generation and Python for filtering shows good integration of tools.
A couple of considerations:
- Ensure the
CHANGELOG_GITHUB_TOKEN
secret is configured in your repository settings- The workflow commits directly to the branch without pull requests, which might bypass branch protection rules if enabled
docs/tutorials/data/binary/mp_data/Ac3Er.json (1)
1-178
: Well-structured materials data file.This JSON file provides comprehensive structural and compositional data for the Ac3Er material with appropriate metadata, lattice parameters, and atomic site information.
docs/conf.py (4)
56-56
: Updated source_suffix format to allow for multiple file types.Converting from string to list format is a good practice that enables future flexibility for supporting multiple document types.
66-66
: Updated copyright year to 2025.Keeping the copyright year current is important for proper documentation.
76-76
: Updated release version to 3.0.2.This aligns with the version updates mentioned in the PR description and changelog workflow.
391-392
: Removing mock modules section.The removal of the mock module configuration suggests the documentation build process has been improved to no longer require these mocks. This simplifies the configuration and likely indicates better dependency management.
.pre-commit-config.yaml (2)
1-5
: Added CI configuration section for pre-commit.This new section provides useful automation settings for pre-commit hooks, including monthly updates and custom commit messages.
8-8
: Updated pre-commit hook versions.All pre-commit hooks have been updated to their latest versions, which is good practice for security and feature improvements. The change in codespell stages from
[commit, commit-msg]
to[pre-commit, commit-msg]
ensures spelling is checked earlier in the workflow.Also applies to: 18-18, 24-24, 52-52, 57-58, 60-60, 65-65, 70-70
docs/tutorials/crystal_space.ipynb (1)
131-138
: Clear user feedback.The new progress and result logs provide helpful feedback. The step-by-step messages are beneficial in tutorials, allowing users to understand each phase of the process.
docs/examples/metallicity.ipynb (1)
1-497
: Well-structured new module demonstration.This notebook comprehensively illustrates the new metallicity module, offering clear examples and explanations. Consider adding a brief explanation of how each metric (e.g., metal fraction, d-block element fraction) is combined into the final metallicity score, so readers grasp the weighting scheme more intuitively.
" text-align: right;\n", | ||
" }\n", | ||
"</style>\n", | ||
"<table border=\"1\" class=\"dataframe\">\n", | ||
" <thead>\n", | ||
" <tr style=\"text-align: right;\">\n", | ||
" <th></th>\n", | ||
" <th>smact_allowed</th>\n", | ||
" <th>mp</th>\n", | ||
" <th>label</th>\n", | ||
" </tr>\n", | ||
" </thead>\n", | ||
" <tbody>\n", | ||
" <tr>\n", | ||
" <th>Cr4C5</th>\n", | ||
" <td>True</td>\n", | ||
" <td>False</td>\n", | ||
" <td>missing</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>Bk3Bi8</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>Cf6F5</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>Hf5Pb4</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>CeHg2</th>\n", | ||
" <td>False</td>\n", | ||
" <td>True</td>\n", | ||
" <td>interesting</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>...</th>\n", | ||
" <td>...</td>\n", | ||
" <td>...</td>\n", | ||
" <td>...</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>Be4Xe3</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>NpSb5</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>Mn4Al</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>Th6Ti7</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" <tr>\n", | ||
" <th>H7Rn</th>\n", | ||
" <td>False</td>\n", | ||
" <td>False</td>\n", | ||
" <td>unlikely</td>\n", | ||
" </tr>\n", | ||
" </tbody>\n", | ||
"</table>\n", | ||
"<p>225879 rows × 3 columns</p>\n", | ||
"</div>" | ||
], | ||
"text/plain": [ | ||
" smact_allowed mp label\n", | ||
"Cr4C5 True False missing\n", | ||
"Bk3Bi8 False False unlikely\n", | ||
"Cf6F5 False False unlikely\n", | ||
"Hf5Pb4 False False unlikely\n", | ||
"CeHg2 False True interesting\n", | ||
"... ... ... ...\n", | ||
"Be4Xe3 False False unlikely\n", | ||
"NpSb5 False False unlikely\n", | ||
"Mn4Al False False unlikely\n", | ||
"Th6Ti7 False False unlikely\n", | ||
"H7Rn False False unlikely\n", | ||
"\n", | ||
"[225879 rows x 3 columns]" | ||
] |
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.
🛠️ Refactor suggestion
Resolve the future warning.
Adapting to the recommended infer_objects(copy=False)
approach or adjusting your fillna usage will help maintain compatibility with future pandas versions.
Sorry |
Description
Updated the filtering_icsd_oxidation_states.ipynb tutorial to reflect the addition of commonality and consensus arguments.
Fixes # (issue)
Type of change
Please delete options that are not relevant.
How Has This Been Tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
All cells in the tutorial run now.
Reviewers
@AntObi
Checklist
Summary by CodeRabbit