Skip to content
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

[WIP] Documentation overhaul #309

Merged
merged 92 commits into from
Nov 19, 2024
Merged

[WIP] Documentation overhaul #309

merged 92 commits into from
Nov 19, 2024

Conversation

AntObi
Copy link
Collaborator

@AntObi AntObi commented Aug 30, 2024

Overhauling the documentation

Description

N/A

Closes #67
Closes #297

Type of change

Please delete options that are not relevant.

  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

TBC

Test Configuration:

  • Python version: 3.10
  • Operating System: macOS

Reviewers

N/A

Checklist:

  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules
  • I have checked my code and corrected any misspellings

Summary by CodeRabbit

  • New Features

    • Enhanced documentation with improved formatting and clarity in the README.md and CONTRIBUTING.md files, facilitating better user navigation and understanding.
    • Introduced support for Jupyter notebooks in documentation, enhancing accessibility for users.
    • Added a new tutorials guide to help users navigate practical applications of the smact library.
    • Expanded the introduction section to outline key features and installation instructions for the smact module.
  • Documentation

    • Standardised section headers and improved overall structure in documentation files, making it easier for users to find relevant information.
    • Updated project metadata in configuration files to reflect branding consistency and enhance discoverability.
    • Changed the HTML theme and added new options for better visual presentation and navigation in documentation.
  • Chores

    • Minor updates to configuration files for improved clarity and consistency.
    • Updated dependencies in project configuration to ensure compatibility with newer features.

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between f7e80f0 and 2369591.

Files selected for processing (14)
  • CONTRIBUTING.md (1 hunks)
  • README.md (3 hunks)
  • docs/conf.py (4 hunks)
  • docs/smact.utils.rst (1 hunks)
  • docs/tutorials/crystal_space_visualisation.ipynb (14 hunks)
  • pyproject.toml (1 hunks)
  • setup.py (2 hunks)
  • smact/init.py (18 hunks)
  • smact/data_loader.py (33 hunks)
  • smact/properties.py (9 hunks)
  • smact/screening.py (17 hunks)
  • smact/tests/test_core.py (11 hunks)
  • smact/tests/test_utils.py (2 hunks)
  • smact/utils/composition.py (4 hunks)
Files skipped from review due to trivial changes (5)
  • CONTRIBUTING.md
  • docs/smact.utils.rst
  • smact/tests/test_core.py
  • smact/tests/test_utils.py
  • smact/utils/composition.py
Files skipped from review as they are similar to previous changes (7)
  • docs/conf.py
  • docs/tutorials/crystal_space_visualisation.ipynb
  • setup.py
  • smact/init.py
  • smact/data_loader.py
  • smact/properties.py
  • smact/screening.py
Additional context used
LanguageTool
README.md

[misspelling] ~24-~24: Possible spelling mistake found.
Context: ...a enough, nature will always confess_ - Roland Coase (from 'How should economists choose?') ...

(EN_MULTITOKEN_SPELLING_TWO)


[style] ~46-~46: Consider a shorter alternative to avoid wordiness.
Context: ...lied to generated lists of compositions in order to screen for particular properties. These...

(IN_ORDER_TO_PREMIUM)


[style] ~66-~66: Would you like to use the Oxford spelling “initialize”? The spelling ‘initialise’ is also correct.
Context: ...es the loading of external data used to initialise the core smact.Element and `smact.Spe...

(OXFORD_SPELLING_Z_NOT_S)


[grammar] ~79-~79: It looks like ‘collections’ doesn’t match ‘a’. Did you mean “a collection” or just “collections”?
Context: ...rediction**: A submodule which contains a collections of tools for predicting dopants. ## Re...

(A_NNS_IN)


[locale-violation] ~113-~113: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ... immediately reflected on the PATH. ## License and attribution Python code and origin...

(LICENCE_LICENSE_NOUN_SINGULAR)


[locale-violation] ~115-~115: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ... data tables are licensed under the MIT License. ## Development notes ### Bugs, featu...

(LICENCE_LICENSE_NOUN_SINGULAR)


[grammar] ~121-~121: The pronoun ‘they’ must be used with a non-third-person form of a verb.
Context: ...n the issue tracker, especially if they helps us improve the docs! For other queries ...

(NON3PRS_VERB)

Additional comments not posted (16)
pyproject.toml (7)

3-3: Updated build system requirements.

The update to the requires field in the [build-system] section to specify a minimum version of setuptools (>=65.0.0) is a good practice. It ensures compatibility with newer features and improvements in the build process.


7-30: Enhanced project metadata.

The addition of a comprehensive [project] section with details such as the project name, version, description, and classification improves the project's discoverability and compliance with packaging standards. The use of classifiers to specify the development status, intended audience, and supported Python versions is particularly beneficial.


41-50: Dependencies specified with version constraints.

Specifying dependencies with version constraints, as seen in the dependencies section, is crucial for maintaining functionality and avoiding conflicts during installation. This approach ensures that the project operates with compatible library versions.


61-76: Optional dependencies categorised.

The creation of new sections for optional dependencies ([project.optional-dependencies]) categorising development and documentation tools allows users to install additional packages as needed without cluttering the main dependencies. This is a good practice for maintainability and user flexibility.


77-77: Semantic versioning configuration.

The introduction of a [tool.semantic_release] section with version_variable set to "setup.py:version" indicates the use of semantic versioning based on tags. This is essential for automated release management and helps in maintaining a clear versioning strategy.


83-154: Linting and formatting tools configuration.

The addition of [tool.ruff] with specific configurations for target Python version, line length, and rules to ignore or enforce enhances code quality and consistency across the project. The detailed setup for linting rules, including exclusions and per-file ignores, is well-thought-out and aligns with best practices for maintaining a clean codebase.


171-274: Spell checking configuration.

The configuration in [tool.codespell] to skip certain files and directories and to ignore specific words (mostly chemical elements) is appropriate given the project's domain. This setup helps avoid false positives while ensuring that genuine spelling mistakes are caught.

README.md (9)

15-15: Title formatting improved.

The reformatting of the title to a Markdown header enhances its visibility and is a good practice for making the README more accessible and professional.


19-20: Documentation and examples links updated.

Updating the links to the documentation and examples improves the accessibility of these resources for users. Ensuring that these links are maintained and accurate is crucial for user engagement.


28-29: Statement of need clearly articulated.

The "Statement of need" section clearly articulates the demand for functional materials and the project's approach to addressing this need. This section is well-written and effectively communicates the project's value proposition.


32-32: Getting started section enhanced.

The "Getting started" section provides clear guidance on how to begin using the project, with links to documentation and examples. This is crucial for new users and contributes to a positive user experience.


38-51: Detailed description of code features.

The "Code features" section provides a detailed description of the core functionalities and modules of the project. This is essential for users to understand what the project offers and how to utilise its features effectively.

Tools
LanguageTool

[style] ~46-~46: Consider a shorter alternative to avoid wordiness.
Context: ...lied to generated lists of compositions in order to screen for particular properties. These...

(IN_ORDER_TO_PREMIUM)


62-80: Comprehensive list of modules.

The "List of modules" section is comprehensive and well-organised, providing a clear overview of the various components of the project. This helps users navigate the project's structure and understand its capabilities.

Tools
LanguageTool

[style] ~66-~66: Would you like to use the Oxford spelling “initialize”? The spelling ‘initialise’ is also correct.
Context: ...es the loading of external data used to initialise the core smact.Element and `smact.Spe...

(OXFORD_SPELLING_Z_NOT_S)


[grammar] ~79-~79: It looks like ‘collections’ doesn’t match ‘a’. Did you mean “a collection” or just “collections”?
Context: ...rediction**: A submodule which contains a collections of tools for predicting dopants. ## Re...

(A_NNS_IN)


81-86: Requirements and installation instructions updated.

The "Requirements" and "Installation" sections have been updated to reflect the current dependencies and installation methods. This is crucial for ensuring that users can set up the project successfully.


117-117: Development notes section provides clear guidance.

The "Development notes" section provides clear guidance on how to report bugs, request features, and contribute to the project. This is important for fostering a collaborative and responsive community.


137-137: References section reformatted.

The "References" section has been reformatted for consistency, ensuring that all citations follow a similar structure. This enhances the professionalism of the document.

Comment on lines +166 to +169
reportPossiblyUnboundVariable = true
reportUnboundVariable = true

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Type checking configuration in Pyright.

Setting typeCheckingMode to "off" in [tool.pyright] might be reconsidered. While it reduces overhead during development, enabling type checking could improve code reliability and catch potential bugs early. It's worth discussing the trade-offs with the team.

Consider enabling type checking to improve code reliability. Here's a proposed change:

-typeCheckingMode = "off"
+typeCheckingMode = "basic"
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
typeCheckingMode = "off"
reportPossiblyUnboundVariable = true
reportUnboundVariable = true
typeCheckingMode = "basic"
reportPossiblyUnboundVariable = true
reportUnboundVariable = true


License and attribution
-----------------------
## License and attribution
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

License section needs localisation correction.

The use of "License" in the section title should be localised to "Licence" to align with British English standards, as the rest of the document uses British English.

Consider localising the term "License" to "Licence" in the section title:

-## License and attribution
+## Licence and attribution
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
## License and attribution
## Licence and attribution
Tools
LanguageTool

[locale-violation] ~113-~113: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ... immediately reflected on the PATH. ## License and attribution Python code and origin...

(LICENCE_LICENSE_NOUN_SINGULAR)


*If you torture the data enough, nature will always confess* - Roland Coase (from 'How should economists choose?')
_If you torture the data enough, nature will always confess_ - Roland Coase (from 'How should economists choose?')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Inspirational quote formatting.

The formatting of the inspirational quote adds a personal touch to the README, though it's worth ensuring that the quote is correctly attributed and spelled. The spelling of "Roland Coase" should be verified.

Please verify the spelling of "Roland Coase" in the quote attribution. If it is incorrect, consider the following correction:

-_If you torture the data enough, nature will always confess_ - Roland Coase (from 'How should economists choose?')
+_If you torture the data enough, nature will always confess_ - Ronald Coase (from 'How should economists choose?')
Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
_If you torture the data enough, nature will always confess_ - Roland Coase (from 'How should economists choose?')
_If you torture the data enough, nature will always confess_ - Ronald Coase (from 'How should economists choose?')
Tools
LanguageTool

[misspelling] ~24-~24: Possible spelling mistake found.
Context: ...a enough, nature will always confess_ - Roland Coase (from 'How should economists choose?') ...

(EN_MULTITOKEN_SPELLING_TWO)

Copy link

codecov bot commented Aug 31, 2024

Codecov Report

Attention: Patch coverage is 65.20619% with 135 lines in your changes missing coverage. Please review.

Project coverage is 72.42%. Comparing base (a7946c9) to head (5167778).
Report is 93 commits behind head on develop.

Files with missing lines Patch % Lines
smact/structure_prediction/prediction.py 21.62% 29 Missing ⚠️
smact/data_loader.py 54.54% 15 Missing ⚠️
smact/__init__.py 77.41% 7 Missing ⚠️
smact/distorter.py 12.50% 7 Missing ⚠️
smact/properties.py 46.15% 7 Missing ⚠️
smact/structure_prediction/mutation.py 68.18% 7 Missing ⚠️
smact/structure_prediction/structure.py 65.00% 7 Missing ⚠️
smact/dopant_prediction/doper.py 64.70% 6 Missing ⚠️
smact/utils/band_gap_simple.py 0.00% 6 Missing ⚠️
...ls/crystal_space/download_compounds_with_mp_api.py 0.00% 6 Missing ⚠️
... and 13 more
Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #309      +/-   ##
===========================================
- Coverage    76.46%   72.42%   -4.05%     
===========================================
  Files           26       30       +4     
  Lines         2265     2415     +150     
===========================================
+ Hits          1732     1749      +17     
- Misses         533      666     +133     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.


🚨 Try these New Features:

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 2369591 and b753b8c.

Files selected for processing (4)
  • docs/examples.rst (1 hunks)
  • docs/examples/valence_electron_count.ipynb (1 hunks)
  • docs/smact.utils.rst (1 hunks)
  • docs/tutorials.rst (1 hunks)
Files skipped from review due to trivial changes (1)
  • docs/examples.rst
Files skipped from review as they are similar to previous changes (2)
  • docs/smact.utils.rst
  • docs/tutorials.rst
Additional comments not posted (1)
docs/examples/valence_electron_count.ipynb (1)

4-18: Well-structured and informative markdown cell.

The markdown cell provides a clear and concise introduction to the concept of valence electron concentration (VEC), its importance, and its calculation. The inclusion of a mathematical formula enhances understanding, and the brief mention of the smact function sets the stage for the subsequent code cell.

Comment on lines 22 to 46
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [
{
"name": "stdout",
"output_type": "stream",
"text": [
"Cu0.5NiAlCoCrFeSi has a VEC of 7.00\n",
"WNbMoTa has a VEC of 5.50\n",
"AlCoCrCu0.5FeNi3 has a VEC of 8.20\n",
"Ca65Mg15Zn20 has a VEC of 4.00\n"
]
}
],
"source": [
"import smact.properties\n",
"\n",
"formulas = [\"Cu0.5NiAlCoCrFeSi\", \"WNbMoTa\", \"AlCoCrCu0.5FeNi3\",\"Ca65Mg15Zn20\"]\n",
"\n",
"valence_electron_concentrations = [smact.properties.valence_electron_count(formula) for formula in formulas]\n",
"\n",
"for formula, vec in zip(formulas, valence_electron_concentrations):\n",
" print(f\"{formula} has a VEC of {vec:.2f}\")"
]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggest adding comments to enhance code clarity.

The code cell correctly demonstrates the calculation of valence electron concentrations using the smact.properties module. However, adding comments to explain each step would enhance readability and maintainability, especially for users unfamiliar with the smact library or the concept of VEC.

Consider adding comments like these:

import smact.properties  # Importing the SMACT properties module

# List of chemical formulas for which VEC will be calculated
formulas = ["Cu0.5NiAlCoCrFeSi", "WNbMoTa", "AlCoCrCu0.5FeNi3", "Ca65Mg15Zn20"]

# Calculate VEC for each formula using a list comprehension
valence_electron_concentrations = [smact.properties.valence_electron_count(formula) for formula in formulas]

# Print the VEC for each formula
for formula, vec in zip(formulas, valence_electron_concentrations):
    print(f"{formula} has a VEC of {vec:.2f}")

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between b753b8c and 0c21a6c.

Files selected for processing (1)
  • docs/conf.py (4 hunks)
Files skipped from review as they are similar to previous changes (1)
  • docs/conf.py

AntObi and others added 2 commits August 31, 2024 09:46
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 0c21a6c and e959684.

Files selected for processing (1)
  • docs/introduction.rst (2 hunks)
Files skipped from review as they are similar to previous changes (1)
  • docs/introduction.rst

Copy link
Contributor

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 0

Outside diff range, codebase verification and nitpick comments (2)
README.md (2)

Line range hint 81-113: Consider adding commas for clarity.

The static analysis tool suggests adding commas in the following places for improved clarity:

  1. Line 89: "The latest stable release can be installed via pip, which will automatically set up other Python packages as required:"

  2. Line 101: "For developer installation, SMACT can be installed from a copy of the source repository (https://github.com/wmd-group/smact); this will be preferred if using experimental code branches."

Apply this diff to add the commas:

-The latest stable release can be installed via pip which will automatically set up other Python packages as required:
+The latest stable release can be installed via pip, which will automatically set up other Python packages as required:

-For developer installation SMACT can be installed from a copy of the source repository (https://github.com/wmd-group/smact); this will be preferred if using experimental code branches.
+For developer installation, SMACT can be installed from a copy of the source repository (https://github.com/wmd-group/smact); this will be preferred if using experimental code branches.
Tools
LanguageTool

[misspelling] ~24-~24: Possible spelling mistake found.
Context: ...a enough, nature will always confess_ - Roland Coase (from 'How should economists choose?') ...

(EN_MULTITOKEN_SPELLING_TWO)


[style] ~46-~46: Consider a shorter alternative to avoid wordiness.
Context: ...lied to generated lists of compositions in order to screen for particular properties. These...

(IN_ORDER_TO_PREMIUM)


[style] ~66-~66: Would you like to use the Oxford spelling “initialize”? The spelling ‘initialise’ is also correct.
Context: ...es the loading of external data used to initialise the core smact.Element and `smact.Spe...

(OXFORD_SPELLING_Z_NOT_S)


[grammar] ~79-~79: It looks like ‘collections’ doesn’t match ‘a’. Did you mean “a collection” or just “collections”?
Context: ...rediction**: A submodule which contains a collections of tools for predicting dopants. ## Re...

(A_NNS_IN)


[uncategorized] ~89-~89: Possible missing comma found.
Context: ...est stable release can be installed via pip which will automatically set up other P...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~101-~101: Possible missing comma found.
Context: ....com/WMD-group/SMACT.git For developer installation SMACT can be installed from a copy of t...

(AI_HYDRA_LEO_MISSING_COMMA)


Line range hint 113-152: Consider making the following changes for consistency and clarity:

  1. Lines 113 and 115: Use the British English spelling "licence" instead of "license" to maintain consistency with the rest of the document.

  2. Line 121: Correct the grammar in the sentence "especially if they helps us improve the docs!" to "especially if they help us improve the docs!".

Apply this diff to make the suggested changes:

-## License and attribution
+## Licence and attribution

-Python code and original data tables are licensed under the MIT License.
+Python code and original data tables are licensed under the MIT Licence.

-Please use the [Issue Tracker](https://github.com/WMD-group/smact/issues) to report bugs or request features in the first instance. While we hope that most questions can be answered by searching [the docs](https://smact.readthedocs.io/en/latest/), we welcome new questions on the issue tracker, especially if they helps us improve the docs! For other queries about any aspect of the code, please contact Anthony Onwuli (maintainer) by [e-mail](mailto:[email protected]).
+Please use the [Issue Tracker](https://github.com/WMD-group/smact/issues) to report bugs or request features in the first instance. While we hope that most questions can be answered by searching [the docs](https://smact.readthedocs.io/en/latest/), we welcome new questions on the issue tracker, especially if they help us improve the docs! For other queries about any aspect of the code, please contact Anthony Onwuli (maintainer) by [e-mail](mailto:[email protected]).
Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between e959684 and 5167778.

Files selected for processing (13)
  • README.md (4 hunks)
  • docs/conf.py (4 hunks)
  • docs/examples/valence_electron_count.ipynb (1 hunks)
  • docs/tutorials/crystal_space.ipynb (1 hunks)
  • docs/tutorials/crystal_space_visualisation.ipynb (14 hunks)
  • pyproject.toml (1 hunks)
  • smact/tests/test_utils.py (3 hunks)
  • smact/utils/band_gap_simple.py (2 hunks)
  • smact/utils/composition.py (4 hunks)
  • smact/utils/crystal_space/init.py (1 hunks)
  • smact/utils/crystal_space/download_compounds_with_mp_api.py (5 hunks)
  • smact/utils/crystal_space/generate_composition_with_smact.py (5 hunks)
  • smact/utils/crystal_space/plot_embedding.py (4 hunks)
Files skipped from review due to trivial changes (2)
  • smact/utils/band_gap_simple.py
  • smact/utils/crystal_space/init.py
Files skipped from review as they are similar to previous changes (5)
  • docs/conf.py
  • docs/examples/valence_electron_count.ipynb
  • docs/tutorials/crystal_space.ipynb
  • docs/tutorials/crystal_space_visualisation.ipynb
  • smact/utils/composition.py
Additional context used
LanguageTool
README.md

[misspelling] ~24-~24: Possible spelling mistake found.
Context: ...a enough, nature will always confess_ - Roland Coase (from 'How should economists choose?') ...

(EN_MULTITOKEN_SPELLING_TWO)


[style] ~46-~46: Consider a shorter alternative to avoid wordiness.
Context: ...lied to generated lists of compositions in order to screen for particular properties. These...

(IN_ORDER_TO_PREMIUM)


[style] ~66-~66: Would you like to use the Oxford spelling “initialize”? The spelling ‘initialise’ is also correct.
Context: ...es the loading of external data used to initialise the core smact.Element and `smact.Spe...

(OXFORD_SPELLING_Z_NOT_S)


[grammar] ~79-~79: It looks like ‘collections’ doesn’t match ‘a’. Did you mean “a collection” or just “collections”?
Context: ...rediction**: A submodule which contains a collections of tools for predicting dopants. ## Re...

(A_NNS_IN)


[uncategorized] ~89-~89: Possible missing comma found.
Context: ...est stable release can be installed via pip which will automatically set up other P...

(AI_HYDRA_LEO_MISSING_COMMA)


[uncategorized] ~101-~101: Possible missing comma found.
Context: ....com/WMD-group/SMACT.git For developer installation SMACT can be installed from a copy of t...

(AI_HYDRA_LEO_MISSING_COMMA)


[locale-violation] ~113-~113: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ... immediately reflected on the PATH. ## License and attribution Python code and origin...

(LICENCE_LICENSE_NOUN_SINGULAR)


[locale-violation] ~115-~115: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ... data tables are licensed under the MIT License. ## Development notes ### Bugs, featu...

(LICENCE_LICENSE_NOUN_SINGULAR)


[grammar] ~121-~121: The pronoun ‘they’ must be used with a non-third-person form of a verb.
Context: ...n the issue tracker, especially if they helps us improve the docs! For other queries ...

(NON3PRS_VERB)

Additional comments not posted (28)
smact/utils/crystal_space/download_compounds_with_mp_api.py (6)

18-18: LGTM!

The updated type hint str | None = None for the mp_api_key parameter enhances clarity by explicitly indicating that the parameter can accept either a string or None as a valid value.


Line range hint 31-43: LGTM!

The addition of the section headers "Args:" and "Returns:" in the docstring improves the documentation structure and readability by clearly separating the description of the function's arguments and return values.


49-49: LGTM!

Consolidating the error message for the missing API key into a single line enhances code conciseness and readability.


65-65: LGTM!

Adding the print statement before the data retrieval process provides clearer feedback during execution, allowing users to track the progress of the download process for each formula_anonymous.


94-95: LGTM!

Using a context manager (with open(...) as f:) for file operations ensures that files are properly closed after writing, which is a best practice for resource management and helps prevent resource leaks.


57-57: LGTM!

Using itertools.combinations_with_replacement instead of itertools.product ensures that the generated combinations of stoichiometric coefficients do not contain duplicates, which helps avoid redundant calculations and improves efficiency.

smact/tests/test_utils.py (3)

66-66: LGTM!

The formatting changes are approved.


84-90: LGTM!

The test_convert_formula method is correctly implemented and covers the expected behaviour of the convert_formula function.


92-94: Verify the purpose of the empty test method.

The test_download_compounds_with_mp_api method is empty and does not contain any test logic. Is this intentional?

If this is a placeholder for a future test, consider adding a TODO comment to document the intended test case and any dependencies required for its execution.

smact/utils/crystal_space/plot_embedding.py (2)

23-23: Docstring addition looks good!

The docstring succinctly describes the purpose of the function, enhancing code readability and maintainability.


70-71: Type hint update and docstring addition look good!

  • The type hints for the reducers and embedding_names parameters have been updated to align with the newer Python type hinting conventions, improving code clarity.
  • The docstring succinctly describes the purpose of the function, enhancing code readability and maintainability.

Also applies to: 77-77

smact/utils/crystal_space/generate_composition_with_smact.py (2)

21-31: Excellent improvements to the function signature and docstring!

The addition of type hints and the expanded docstring significantly enhance the clarity and maintainability of the code. The changes provide clear guidance on the expected types of the function parameters and the return value, making it easier for future developers to understand and use the function correctly.

The core logic of the function remains unaltered, ensuring that the existing functionality is preserved.


Line range hint 39-138: Great job on enhancing the function signature, docstring, and overall readability!

The introduction of type hints for num_processes and save_path parameters clarifies the expected types, allowing for better type checking and reducing potential type-related issues. The updated docstring provides a clearer explanation of the function's purpose and the types of its parameters, making it more user-friendly for other developers.

The minor formatting adjustments, such as consolidating list comprehensions and removing unnecessary line breaks, improve the overall readability of the code without altering its functionality. These changes make the code more concise and easier to follow.

The core logic and control flow of the function remain unaffected, ensuring that the existing behaviour is preserved.

pyproject.toml (5)

3-3: LGTM!

The change to require a minimum version of setuptools is a good practice to ensure compatibility with newer features and improvements in the build process.


6-52: LGTM!

The addition of the [project] section significantly enhances the project's configuration by providing essential metadata. This includes the project name, version, description, and other details that aid in discoverability, compliance with packaging standards, and maintainability. The specified dependencies ensure that the project operates with compatible library versions, which is crucial for maintaining functionality and avoiding conflicts during installation.


60-83: LGTM!

The categorisation of optional dependencies into separate sections, such as dev and docs, is a good practice. It allows users to install additional packages as needed for development or documentation purposes without cluttering the main dependencies. This enhances the project's usability and maintainability by keeping the core dependencies focused and relevant.


85-86: LGTM!

The addition of the [tool.semantic_release] section is essential for automated release management. It specifies the version variable and the source of the version information (tags). This configuration enables the project to follow semantic versioning principles, which helps in maintaining a consistent and meaningful version numbering scheme. Automated release management based on semantic versioning simplifies the release process and improves the overall maintainability of the project.


88-162: LGTM!

The addition of the [tool.ruff] section enhances code quality and consistency across the project. It specifies the target Python version, line length limit, and a comprehensive list of linting rules to ignore or enforce. This configuration helps maintain a consistent coding style, identifies potential issues, and promotes best practices. The use of a linting tool like Ruff improves the overall maintainability and readability of the codebase.

README.md (10)

Line range hint 1-13: LGTM!

The badges are correctly formatted and provide useful information about the project.

Tools
LanguageTool

[locale-violation] ~3-~3: License must be spelled with a “c” when used as a noun in British English. Use “licence”.
Context: ...eadthedocs.org/en/latest/?badge=latest) [![License: MIT](https://img.shields.io/badge/Lice...

(LICENCE_LICENSE_NOUN_SINGULAR)


15-15: LGTM!

The project title is correctly formatted as a top-level heading.


19-20: LGTM!

The links to the project's documentation and examples are correctly formatted and provide easy access to these resources.


22-22: LGTM!

The image showcasing the SMACT interface is correctly formatted and provides a visual representation of the project.


24-24: Verify the spelling of the quote attribution.

The spelling of "Roland Coase" in the quote attribution should be verified. If it is incorrect, consider the following correction:

-_If you torture the data enough, nature will always confess_ - Roland Coase (from 'How should economists choose?')
+_If you torture the data enough, nature will always confess_ - Ronald Coase (from 'How should economists choose?')
Tools
LanguageTool

[misspelling] ~24-~24: Possible spelling mistake found.
Context: ...a enough, nature will always confess_ - Roland Coase (from 'How should economists choose?') ...

(EN_MULTITOKEN_SPELLING_TWO)


28-29: LGTM!

The description effectively communicates the project's purpose and capabilities.


30-31: LGTM!

The GIF showcasing the usage of the SMACT code is correctly formatted and provides a visual demonstration of how the code can be used.


32-38: LGTM!

The information provided on how to get started with the project is clear and helpful. The links to the documentation, examples, and tutorials are correctly formatted and provide easy access to these resources.


40-62: LGTM!

The list of features provides a comprehensive overview of the capabilities of the SMACT code. The links to the relevant documentation for each feature are correctly formatted and provide easy access to more detailed information.

Tools
LanguageTool

[style] ~46-~46: Consider a shorter alternative to avoid wordiness.
Context: ...lied to generated lists of compositions in order to screen for particular properties. These...

(IN_ORDER_TO_PREMIUM)


64-80: LGTM!

The list of modules and their descriptions provide a clear overview of the structure and functionality of the SMACT library.

Tools
LanguageTool

[style] ~66-~66: Would you like to use the Oxford spelling “initialize”? The spelling ‘initialise’ is also correct.
Context: ...es the loading of external data used to initialise the core smact.Element and `smact.Spe...

(OXFORD_SPELLING_Z_NOT_S)


[grammar] ~79-~79: It looks like ‘collections’ doesn’t match ‘a’. Did you mean “a collection” or just “collections”?
Context: ...rediction**: A submodule which contains a collections of tools for predicting dopants. ## Re...

(A_NNS_IN)

@AntObi AntObi merged commit 1636ad7 into develop Nov 19, 2024
14 of 16 checks passed
@AntObi AntObi deleted the docs_updates branch November 19, 2024 23:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file docs enhancement feature python Pull requests that update Python code refactor WIP Work in progress
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Move to ruff for linting and formating Running examples from a browser
7 participants