Skip to content

Fortran revise#569

Merged
LecrisUT merged 3 commits intospglib:developfrom
krefson:fortran-revise
Apr 24, 2025
Merged

Fortran revise#569
LecrisUT merged 3 commits intospglib:developfrom
krefson:fortran-revise

Conversation

@krefson
Copy link
Copy Markdown
Contributor

@krefson krefson commented Apr 23, 2025

No description provided.

LecrisUT
LecrisUT approved these changes Apr 23, 2025
View reviewed changes
Copy link
Copy Markdown
Collaborator

@LecrisUT LecrisUT left a comment

Choose a reason for hiding this comment

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

Awesome stuff. Could you fix the pre-commit issues?

@krefson
Copy link
Copy Markdown
Contributor Author

krefson commented Apr 23, 2025

The pre-commit messages refer to files I have not touched. Please advise what is wrong here.

@LecrisUT
Copy link
Copy Markdown
Collaborator

LecrisUT commented Apr 23, 2025

They have probably changed clang-format behaviour in the ubuntu-latest image. I will make a PR to fix that.

@codecov
Copy link
Copy Markdown

codecov bot commented Apr 23, 2025 •
edited
Loading

Codecov Report

Attention: Patch coverage is 35.29412% with 33 lines in your changes missing coverage. Please review.

Project coverage is 74.09%. Comparing base (a9ef4e9) to head (6287a16).
Report is 18 commits behind head on develop.

Files with missing lines Patch % Lines
fortran/spglib_f08.F90 35.29% 33 Missing ⚠️
Additional details and impacted files 
@@             Coverage Diff             @@
##           develop     #569      +/-   ##
===========================================
- Coverage    74.32%   74.09%   -0.24%     
===========================================
  Files           25       25              
  Lines         8071     8049      -22     
  Branches      1693     1675      -18     
===========================================
- Hits          5999     5964      -35     
- Misses        1562     1577      +15     
+ Partials       510      508       -2
Flag Coverage Δ
c_api 68.67% <ø> (ø)
fortran_api 50.96% <85.71%> (+0.27%) ⬆️
python_api 66.96% <ø> (ø)
unit_tests 11.71% <0.00%> (+0.03%) ⬆️

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
Copy link
Copy Markdown
Collaborator

LecrisUT commented Apr 23, 2025

Could you add a line in the ChangeLog.md about exposing spg_get_spacegroup_type_from_symmetry?

@krefson
Copy link
Copy Markdown
Contributor Author

krefson commented Apr 23, 2025

I have added an entry to the Changelog.

The code coverage report is mysterious, and I think wrong. It reports that my new variant of c_to_f_string is not being executed but it absolutely must be to run the test suite. (I get 100% pass). I have a hypothesis that the function is now so compact that it is being inlined, and the character copy operations have been optimised away entirely. It's not clear how code coverage test would cope with that,.

@LecrisUT
Copy link
Copy Markdown
Collaborator

LecrisUT commented Apr 23, 2025

The code coverage report is mysterious, and I think wrong.

Yeah, you can ignore it, it is random fluctuation, didn't look into what setting needs to be added to avoid the error report, or disable it altogether. We mostly want an overview from it.

@krefson krefson closed this Apr 23, 2025
@krefson krefson reopened this Apr 23, 2025
@krefson
Copy link
Copy Markdown
Contributor Author

krefson commented Apr 24, 2025

What is the next step? Is there a further process to get this merged?

@LecrisUT LecrisUT enabled auto-merge (squash) April 24, 2025 07:18
@LecrisUT LecrisUT merged commit ff295fa into spglib:develop Apr 24, 2025
42 of 49 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@LecrisUT LecrisUT LecrisUT approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@krefson @LecrisUT