Skip to content

Simplify python dataset construction#578

Merged
lan496 merged 1 commit intospglib:developfrom
LecrisUT:feat/python-dict
Nov 11, 2025
Merged

Simplify python dataset construction#578
lan496 merged 1 commit intospglib:developfrom
LecrisUT:feat/python-dict

Conversation

@LecrisUT
Copy link
Copy Markdown
Collaborator

@LecrisUT LecrisUT commented May 10, 2025

For now we make the dict from the python side directly. A further simplification would be to move the class inside as a C++ class or making it a thin wrapper of one. That I will probably postpone until we get enough familiarity with the tools.

Depends-on: #576

@codecov
Copy link
Copy Markdown

codecov bot commented May 10, 2025 •
edited
Loading

Codecov Report

❌ Patch coverage is 96.80851% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 74.51%. Comparing base (a10e6e5) to head (79147bf).
⚠️ Report is 68 commits behind head on develop.

Files with missing lines Patch % Lines
python/py_bindings.cpp 96.80% 2 Missing and 1 partial ⚠️
Additional details and impacted files 
@@           Coverage Diff            @@
##           develop     #578   +/-   ##
========================================
  Coverage    74.51%   74.51%           
========================================
  Files           26       26           
  Lines         7895     7899    +4     
  Branches      1638     1635    -3     
========================================
+ Hits          5883     5886    +3     
  Misses        1534     1534           
- Partials       478      479    +1
Flag Coverage Δ
c_api 69.45% <ø> (ø)
fortran_api 52.96% <ø> (ø)
python_api 67.72% <96.80%> (+<0.01%) ⬆️
unit_tests 11.68% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

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

@LecrisUT LecrisUT mentioned this pull request May 10, 2025
Merged
3 tasks
@lan496
Copy link
Copy Markdown
Member

lan496 commented May 11, 2025

Can you change the base branch from develop to #576 for ease of review?
image

@LecrisUT
Copy link
Copy Markdown
Collaborator Author

LecrisUT commented May 12, 2025

Can you change the base branch from develop to #576 for ease of review?

Unfortunately that is not possible because #576 source branch is from my fork. Maybe you can come back to this after it is merged or check the individual commits (this one only differs on the last commit)

@LecrisUT LecrisUT force-pushed the feat/python-dict branch from c349694 to 9202b8c Compare May 12, 2025 07:25
@lan496
Copy link
Copy Markdown
Member

lan496 commented May 12, 2025

Unfortunately that is not possible because #576 source branch is from my fork

Sorry, I've missed this point

@LecrisUT LecrisUT force-pushed the feat/python-dict branch from 9202b8c to 0727fee Compare May 12, 2025 10:05
@LecrisUT LecrisUT mentioned this pull request Jun 2, 2025
Closed
@LecrisUT LecrisUT added this to the 2.7 milestone Jun 2, 2025
@LecrisUT LecrisUT requested review from atztogo and lan496 November 9, 2025 21:47
lan496
lan496 reviewed Nov 10, 2025
View reviewed changes
lan496
lan496 approved these changes Nov 10, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@lan496 lan496 left a comment

Choose a reason for hiding this comment

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

LGTM!

@lan496
Copy link
Copy Markdown
Member

lan496 commented Nov 10, 2025

How to handle the issue in #616 ? I think it is fine to bypass and merge this PR although the bilbao's link is also broken in this PR

@LecrisUT
Copy link
Copy Markdown
Collaborator Author

LecrisUT commented Nov 11, 2025

How to handle the issue in #616 ? I think it is fine to bypass and merge this PR although the bilbao's link is also broken in this PR

Yes, just double check that it is the only faiand bypass it for now

@lan496 lan496 merged commit 21e8a26 into spglib:develop Nov 11, 2025
46 of 48 checks passed
@LecrisUT LecrisUT deleted the feat/python-dict branch November 19, 2025 11:06
lan496 added a commit that referenced this pull request Nov 20, 2025
@lan496
Throw python exceptions (#579)
170ff8c 
TODO:
- [x] Make the library throw exceptions
- [x] Add a compatibility for the old error handling
- [x] Add more checks in the C++ code

Depends-on: #578 
Closes #582
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@atztogo atztogo atztogo approved these changes

@lan496 lan496 lan496 approved these changes

Assignees

@lan496 lan496

Labels

None yet

Projects

None yet

Milestone

2.7

Development

Successfully merging this pull request may close these issues.

3 participants

@LecrisUT @lan496 @atztogo