Skip to content

Conversation

KingaMas
Copy link
Member

@KingaMas KingaMas commented Jul 9, 2025

Description

Changes to the smact.utils.oxidation module allowing for the creation of custom oxidation states lists

Type of change

Please delete options that are not relevant.

  • [ x ] New feature (non-breaking change which adds functionality)

How Has This Been Tested?

-pytest

Test Configuration:

  • Python version: 3.13
  • Operating System: MacOS

Reviewers

@mention individuals who you specifically want to involve in the discussion for this pull request and mention why they are needed in the discussion/why they are needed to review the pull request.

Checklist

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

Summary by CodeRabbit

Summary by CodeRabbit

New Features

  • Added support for generating compositions using custom oxidation states files.
  • Extended oxidation state lookup to allow retrieval of all custom oxidation states at once.

Improvements

  • Enhanced oxidation state filtering options for composition validity checks, allowing finer control over which states are considered.
  • Improved documentation for several functions, clarifying usage and return formats.
  • Optimised data loading by skipping irrelevant lines and clarified warning placement.

Dependency Updates

  • Updated multiple Python package versions for improved compatibility and stability.

Chores

  • Updated GitHub Actions workflows to use the latest setup action versions.
  • Minor style and formatting clean-up in documentation.

KingaMas and others added 30 commits February 25, 2025 15:16
Updating tutorials
Sync develop branch with latest commits on master
Bumps [astral-sh/setup-uv](https://github.com/astral-sh/setup-uv) from 5 to 6.
- [Release notes](https://github.com/astral-sh/setup-uv/releases)
- [Commits](astral-sh/setup-uv@v5...v6)

---
updated-dependencies:
- dependency-name: astral-sh/setup-uv
  dependency-version: '6'
  dependency-type: direct:production
  update-type: version-update:semver-major
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [pydash](https://github.com/dgilland/pydash) from 8.0.4 to 8.0.5.
- [Changelog](https://github.com/dgilland/pydash/blob/develop/CHANGELOG.rst)
- [Commits](dgilland/pydash@v8.0.4...v8.0.5)

---
updated-dependencies:
- dependency-name: pydash
  dependency-version: 8.0.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [importlib-metadata](https://github.com/python/importlib_metadata) from 8.5.0 to 8.7.0.
- [Release notes](https://github.com/python/importlib_metadata/releases)
- [Changelog](https://github.com/python/importlib_metadata/blob/main/NEWS.rst)
- [Commits](python/importlib_metadata@v8.5.0...v8.7.0)

---
updated-dependencies:
- dependency-name: importlib-metadata
  dependency-version: 8.7.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [pymatgen](https://github.com/materialsproject/pymatgen) from 2025.3.10 to 2025.4.24.
- [Release notes](https://github.com/materialsproject/pymatgen/releases)
- [Changelog](https://github.com/materialsproject/pymatgen/blob/master/docs/CHANGES.md)
- [Commits](materialsproject/pymatgen@v2025.3.10...v2025.4.24)

---
updated-dependencies:
- dependency-name: pymatgen
  dependency-version: 2025.4.24
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps mp-api from 0.45.3 to 0.45.5.

---
updated-dependencies:
- dependency-name: mp-api
  dependency-version: 0.45.5
  dependency-type: direct:production
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <support@github.com>
Deleted a .png file with a picture of snoopy the dog
Bumps [setuptools](https://github.com/pypa/setuptools) from 75.6.0 to 78.1.1.
- [Release notes](https://github.com/pypa/setuptools/releases)
- [Changelog](https://github.com/pypa/setuptools/blob/main/NEWS.rst)
- [Commits](pypa/setuptools@v75.6.0...v78.1.1)

---
updated-dependencies:
- dependency-name: setuptools
  dependency-version: 78.1.1
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [tornado](https://github.com/tornadoweb/tornado) from 6.4.2 to 6.5.1.
- [Changelog](https://github.com/tornadoweb/tornado/blob/master/docs/releases.rst)
- [Commits](tornadoweb/tornado@v6.4.2...v6.5.1)

---
updated-dependencies:
- dependency-name: tornado
  dependency-version: 6.5.1
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [tqdm](https://github.com/tqdm/tqdm) from 4.66.4 to 4.67.1.
- [Release notes](https://github.com/tqdm/tqdm/releases)
- [Commits](tqdm/tqdm@v4.66.4...v4.67.1)

---
updated-dependencies:
- dependency-name: tqdm
  dependency-version: 4.67.1
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [threadpoolctl](https://github.com/joblib/threadpoolctl) from 3.5.0 to 3.6.0.
- [Release notes](https://github.com/joblib/threadpoolctl/releases)
- [Changelog](https://github.com/joblib/threadpoolctl/blob/master/CHANGES.md)
- [Commits](joblib/threadpoolctl@3.5.0...3.6.0)

---
updated-dependencies:
- dependency-name: threadpoolctl
  dependency-version: 3.6.0
  dependency-type: direct:production
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [requests](https://github.com/psf/requests) from 2.32.2 to 2.32.4.
- [Release notes](https://github.com/psf/requests/releases)
- [Changelog](https://github.com/psf/requests/blob/main/HISTORY.md)
- [Commits](psf/requests@v2.32.2...v2.32.4)

---
updated-dependencies:
- dependency-name: requests
  dependency-version: 2.32.4
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Bumps [urllib3](https://github.com/urllib3/urllib3) from 2.2.2 to 2.5.0.
- [Release notes](https://github.com/urllib3/urllib3/releases)
- [Changelog](https://github.com/urllib3/urllib3/blob/main/CHANGES.rst)
- [Commits](urllib3/urllib3@2.2.2...2.5.0)

---
updated-dependencies:
- dependency-name: urllib3
  dependency-version: 2.5.0
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <support@github.com>
Copy link
Contributor

coderabbitai bot commented Jul 9, 2025

Walkthrough

This update introduces support for custom oxidation states in composition generation and filtering, enhances the flexibility of oxidation state selection in validity checks, and updates dependency versions. It also updates CI workflows to use the latest setup action, removes trailing blank lines in documentation, and includes minor code clarity improvements.

Changes

File(s) Change Summary
.github/workflows/ci.yml
.github/workflows/publish-to-pypi.yml
.github/workflows/update-changelog.yml
Updated GitHub Actions workflows to use astral-sh/setup-uv@v6 instead of v5.
README.md Removed a single trailing blank line from a bullet list.
pyproject.toml
requirements.txt
requirements/requirements-py310.txt
Updated versions for mp-api, pymatgen, importlib-metadata, pydash, requests, and urllib3 dependencies.
smact/data_loader.py Improved handling of non-data lines, clarified docstrings, and added support for "all" in custom oxidation states lookup.
smact/screening.py Enhanced oxidation state filtering in smact_validity, added new parameters, clarified docstrings, and simplified logic.
smact/utils/crystal_space/generate_composition_with_smact.py Added generate_composition_with_smact_custom for supporting custom oxidation states; updated docs and imports.
smact/utils/oxidation.py Minor formatting changes with no functional impact.
smact/tests/test_utils.py Added tests for generate_composition_with_smact_custom verifying output and file saving.

Sequence Diagram(s)

sequenceDiagram
    participant User
    participant generate_composition_with_smact_custom
    participant lookup_element_oxidation_states_custom
    participant smact_filter

    User->>generate_composition_with_smact_custom: Call with custom oxidation_states_set
    generate_composition_with_smact_custom->>lookup_element_oxidation_states_custom: Load custom oxidation states
    generate_composition_with_smact_custom->>smact_filter: Filter generated compositions using custom oxidation states
    smact_filter-->>generate_composition_with_smact_custom: Return filtered compositions
    generate_composition_with_smact_custom-->>User: Return DataFrame/results
Loading
sequenceDiagram
    participant User
    participant smact_validity
    participant ICSD24OxStatesFilter

    User->>smact_validity: Call with parameters (incl. consensus, commonality)
    smact_validity->>ICSD24OxStatesFilter: Filter oxidation states as per parameters
    ICSD24OxStatesFilter-->>smact_validity: Return filtered oxidation states
    smact_validity-->>User: Return validity (True/False)
Loading

Possibly related PRs

  • WMD-group/SMACT#432: Also modifies smact_validity for oxidation state filtering, indicating direct code-level overlap.
  • WMD-group/SMACT#431: Adds support for custom oxidation states in data loading and composition generation, closely related to this update.
  • WMD-group/SMACT#346: Modifies oxidation states handling and function signatures in the same modules, showing strong connection.

Suggested labels

enhancement, tests, docs

Suggested reviewers

  • hspark1212

Poem

In the warren of code, a new path appears,
With custom states hopping through data fields clear.
Filters grow clever, dependencies bloom,
CI workflows updated, no trailing blank gloom.
SMACT leaps forward with each thoughtful tweak,
The rabbit applauds—new features unique!
🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b63e63e and e37c28c.

📒 Files selected for processing (2)
  • requirements.txt (6 hunks)
  • requirements/requirements-py310.txt (2 hunks)
✅ Files skipped from review due to trivial changes (1)
  • requirements/requirements-py310.txt
🚧 Files skipped from review as they are similar to previous changes (1)
  • requirements.txt
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: test (3.12, windows-latest)
  • GitHub Check: test (3.13, ubuntu-latest)
  • GitHub Check: test (3.12, macos-latest)
  • GitHub Check: test (3.13, macos-latest)
  • GitHub Check: test (3.11, macos-latest)
  • GitHub Check: test (3.12, ubuntu-latest)
  • GitHub Check: test (3.10, macos-latest)
  • GitHub Check: test (3.11, ubuntu-latest)
  • GitHub Check: test (3.11, windows-latest)
  • GitHub Check: test (3.10, windows-latest)
  • GitHub Check: test (3.10, ubuntu-latest)
✨ Finishing Touches
🧪 Generate unit tests
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch develop

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Explain this complex logic.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai explain this code block.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and explain its main purpose.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai auto-generate unit tests to generate unit tests for this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai or @coderabbitai title anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@KingaMas KingaMas requested a review from AntObi July 9, 2025 13:11
Copy link

codecov bot commented Jul 9, 2025

Codecov Report

❌ Patch coverage is 94.80519% with 4 lines in your changes missing coverage. Please review.
✅ Project coverage is 80.66%. Comparing base (253d325) to head (e37c28c).
⚠️ Report is 64 commits behind head on master.

Files with missing lines Patch % Lines
smact/screening.py 85.71% 3 Missing ⚠️
smact/data_loader.py 80.00% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #436      +/-   ##
==========================================
+ Coverage   80.03%   80.66%   +0.63%     
==========================================
  Files          33       33              
  Lines        2805     2871      +66     
==========================================
+ Hits         2245     2316      +71     
+ Misses        560      555       -5     

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@KingaMas KingaMas changed the title Develop Adding changes from develop Jul 9, 2025
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: 4

🔭 Outside diff range comments (1)
smact/screening.py (1)

510-511: Remove redundant condition check.

The condition oxidation_states_set == "icsd24" or oxidation_states_set is None will never have oxidation_states_set is None as true because it's already handled in the if block starting at line 491.

Apply this diff to fix the redundant condition:

-    elif oxidation_states_set == "icsd24" or oxidation_states_set is None:
+    elif oxidation_states_set == "icsd24":
         ox_combos = [el.oxidation_states_icsd24 for el in smact_elems]
🧹 Nitpick comments (3)
.github/workflows/ci.yml (1)

26-29: Matrix step: watch for Windows + Python 3.13 re-enable

No functional issue with the v6 bump, but the comment above still mentions a “temporary” exclusion for Windows/3.13. When setup-uv fully supports 3.13 on Windows, remember to drop the exclusion to restore coverage parity.

requirements.txt (1)

382-386: pymatgen 2025.4.24: forward-compat with NumPy 2.0

The pinned build is NumPy-2-ready but still flags numpy.bool8 deprecations. Ensure the upper bound numpy<3 in pyproject.toml is retained until full migration.

smact/screening.py (1)

491-503: Consider extracting oxidation state filtering logic.

The logic for filtering oxidation states when oxidation_states_set is None adds significant complexity to this function. Consider extracting it into a helper function for better readability and testability.

def _get_filtered_oxidation_states(smact_elems, consensus, include_zero, commonality):
    """Get filtered oxidation states for elements using ICSD24OxStatesFilter."""
    ox_filter = ICSD24OxStatesFilter()
    filtered_df = ox_filter.filter(consensus=consensus, include_zero=include_zero, commonality=commonality)
    oxidation_dict = {
        row["element"]: [int(x) for x in row["oxidation_state"].split()] 
        for _, row in filtered_df.iterrows()
    }
    
    ox_combos = []
    for el in smact_elems:
        ox_el = oxidation_dict.get(el.symbol, None)
        if ox_el is not None:
            ox_combos.append(ox_el)
        else:
            return None
    return ox_combos

Then use it as:

if oxidation_states_set is None:
    ox_combos = _get_filtered_oxidation_states(smact_elems, consensus, include_zero, commonality)
    if ox_combos is None:
        return False
📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 253d325 and 36b38b4.

📒 Files selected for processing (10)
  • .github/workflows/ci.yml (1 hunks)
  • .github/workflows/publish-to-pypi.yml (1 hunks)
  • .github/workflows/update-changelog.yml (1 hunks)
  • README.md (0 hunks)
  • pyproject.toml (1 hunks)
  • requirements.txt (4 hunks)
  • smact/data_loader.py (3 hunks)
  • smact/screening.py (5 hunks)
  • smact/utils/crystal_space/generate_composition_with_smact.py (3 hunks)
  • smact/utils/oxidation.py (3 hunks)
💤 Files with no reviewable changes (1)
  • README.md
🧰 Additional context used
🧬 Code Graph Analysis (1)
smact/screening.py (2)
smact/__init__.py (2)
  • neutral_ratios (508-547)
  • _gcd_recursive (450-455)
smact/utils/oxidation.py (2)
  • ICSD24OxStatesFilter (13-210)
  • filter (26-81)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (11)
  • GitHub Check: test (3.12, windows-latest)
  • GitHub Check: test (3.13, ubuntu-latest)
  • GitHub Check: test (3.13, macos-latest)
  • GitHub Check: test (3.11, ubuntu-latest)
  • GitHub Check: test (3.10, windows-latest)
  • GitHub Check: test (3.12, macos-latest)
  • GitHub Check: test (3.12, ubuntu-latest)
  • GitHub Check: test (3.11, macos-latest)
  • GitHub Check: test (3.10, macos-latest)
  • GitHub Check: test (3.11, windows-latest)
  • GitHub Check: test (3.10, ubuntu-latest)
🔇 Additional comments (9)
.github/workflows/update-changelog.yml (1)

38-41: Confirm action-v6 compatibility and cache behaviour

setup-uv@v6 introduces a new caching strategy and a stricter Python discovery. Double-check that (a) the existing uv run invocations still pick up the correct interpreter and (b) the cache key is invalidated on lock-file changes, otherwise stale wheels may leak into the release build.

.github/workflows/publish-to-pypi.yml (1)

18-21: Version bump looks safe but verify release artefact names

The v6 action now defaults to a dist/* pattern that may differ when uv build produces tagged wheels (e.g. *.whl, *.tar.gz). Ensure actions/upload-artifact still finds all artefacts after the bump.

requirements.txt (3)

126-132: importlib-metadata 8.7 → 8.7.0: check deprecated aliases

Minor, but 8.7 removes a few legacy entry-point helpers. If any runtime code imports importlib_metadata.EntryPoint, tests might miss it.


226-230: mp-api 0.45.5 aligns with upstream, good

Upgrade matches the API-breaking release notes (new robocrys schema). Implementation in smact.utils.oxidation is untouched, so LGTM.


372-376: pydash 8.0.4 → 8.0.5: watch for tightened typing

8.0.5 hardened a handful of type hints; pyright may now raise when Any is passed.

smact/data_loader.py (2)

45-50: Good performance optimization!

The addition of checking for digits before yielding lines is a sensible optimization that will skip element entries without oxidation states, reducing unnecessary processing.


290-294: Safe: _el_ox_states_custom is always initialised before the “all” branch

  • The initialisation logic at line 278 in smact/data_loader.py assigns _el_ox_states_custom whenever an oxidation_states_set is provided or if it was previously None.
  • The only invocation of lookup_element_oxidation_states_custom("all", ...) in smact/utils/crystal_space/generate_composition_with_smact.py passes a non-null oxidation_states_set, ensuring _el_ox_states_custom cannot be None.
    No further changes are required.
smact/utils/crystal_space/generate_composition_with_smact.py (1)

215-215: Clarify the Francium electronegativity threshold.

The code filters elements based on having Pauling electronegativity greater than or equal to Francium's. This seems like an arbitrary threshold that should be documented or made configurable.

Why is Francium's electronegativity (0.7) used as the threshold? This excludes only Francium itself. Consider:

  1. Making this threshold a parameter
  2. Adding a comment explaining the rationale
  3. Using a more explicit filter if the intention is just to exclude Francium
smact/screening.py (1)

395-395: Good optimization using generator expression.

Using a generator expression instead of a list comprehension is more memory-efficient when dealing with potentially large sets of oxidation states.

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
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between e58f872 and e068d34.

📒 Files selected for processing (1)
  • smact/tests/test_utils.py (1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
smact/tests/test_utils.py (1)
smact/utils/crystal_space/generate_composition_with_smact.py (1)
  • generate_composition_with_smact (42-145)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (9)
  • GitHub Check: test (3.13, ubuntu-latest)
  • GitHub Check: test (3.12, ubuntu-latest)
  • GitHub Check: test (3.11, ubuntu-latest)
  • GitHub Check: test (3.10, windows-latest)
  • GitHub Check: test (3.12, windows-latest)
  • GitHub Check: test (3.12, macos-latest)
  • GitHub Check: test (3.11, macos-latest)
  • GitHub Check: test (3.10, ubuntu-latest)
  • GitHub Check: test (3.11, windows-latest)

@AntObi AntObi merged commit a1c5882 into master Jul 11, 2025
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants