Skip to content

Extend unit tests for get_primitive_cell()#485

Merged
jan-janssen merged 4 commits intostandardizefrom
extend-primitive-cell-tests-17824912344481522576
Apr 10, 2026
Merged

Extend unit tests for get_primitive_cell()#485
jan-janssen merged 4 commits intostandardizefrom
extend-primitive-cell-tests-17824912344481522576

Conversation

@jan-janssen
Copy link
Copy Markdown
Member

This PR extends the unit tests for the get_primitive_cell() function in src/structuretoolkit/analyse/symmetry.py.

The new tests in tests/test_analyse_symmetry.py verify:

  1. That a ValueError with the message "Can only symmetrize periodic structures." is raised when the input structure does not have all periodic boundary conditions set to True.
  2. That a warning "Custom arrays {'...'} do not carry over to new structure!" is logged when the input structure contains custom arrays that are not preserved in the primitive cell.

These additions ensure better coverage of the validation and notification logic within get_primitive_cell().


PR created automatically by Jules for task 17824912344481522576 started by @jan-janssen

Added test cases to cover:
- Periodic boundary condition validation: ensuring ValueError is raised if structure is not periodic.
- Custom arrays warning: ensuring a warning is logged when custom arrays are present in the source structure.

Co-authored-by: jan-janssen <3854739+jan-janssen@users.noreply.github.com>
@google-labs-jules
Copy link
Copy Markdown

👋 Jules, reporting for duty! I'm here to lend a hand with this pull request.

When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down.

I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job!

For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with @jules. You can find this option in the Pull Request section of your global Jules UI settings. You can always switch back!

New to Jules? Learn more at jules.google/docs.


For security, I will only act on instructions from the user who triggered this task.

Copilot AI review requested due to automatic review settings April 10, 2026 20:09
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Apr 10, 2026

Important

Review skipped

Auto reviews are disabled on base/target branches other than the default branch.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: a58e643e-dcb3-4eaf-9e21-3c76cd7e9570

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

  • 🔍 Trigger review
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch extend-primitive-cell-tests-17824912344481522576

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 10, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 82.94%. Comparing base (012dadd) to head (ba311dd).
⚠️ Report is 1 commits behind head on standardize.

Additional details and impacted files
@@               Coverage Diff               @@
##           standardize     #485      +/-   ##
===============================================
+ Coverage        82.84%   82.94%   +0.10%     
===============================================
  Files               26       26              
  Lines             1859     1859              
===============================================
+ Hits              1540     1542       +2     
+ Misses             319      317       -2     

☔ 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.

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR extends unit test coverage around Symmetry.get_primitive_cell() to ensure expected validation and logging behavior is exercised in structuretoolkit.analyse.symmetry.

Changes:

  • Added a unit test asserting a ValueError is raised when the input structure is not fully periodic (pbc not all True).
  • Added a unit test asserting a warning is logged when custom Atoms.arrays entries won’t be preserved in the primitive cell.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

structure = bulk("Al", cubic=True)
structure.pbc = [True, True, False]
sym = stk.analyse.get_symmetry(structure=structure)
with self.assertRaisesRegex(ValueError, "Can only symmetrize periodic structures."):
Copy link

Copilot AI Apr 10, 2026

Choose a reason for hiding this comment

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

assertRaisesRegex treats the pattern as a regular expression; the trailing . in the message currently matches any character. If the intent is to assert the exact message (including the period), consider escaping/anchoring the pattern (e.g., escape the dot and use ^...$) so the test can’t pass on slightly different messages.

Suggested change
with self.assertRaisesRegex(ValueError, "Can only symmetrize periodic structures."):
with self.assertRaisesRegex(
ValueError, r"^Can only symmetrize periodic structures\.$"
):

Copilot uses AI. Check for mistakes.
@jan-janssen jan-janssen merged commit 092cd63 into standardize Apr 10, 2026
19 checks passed
@jan-janssen jan-janssen deleted the extend-primitive-cell-tests-17824912344481522576 branch April 10, 2026 20:33
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