Skip to content

Removed deprecated Unicode arguments - #149

Merged
ohno merged 8 commits into
mainfrom
issue/143-2
Aug 6, 2026
Merged

Removed deprecated Unicode arguments#149
ohno merged 8 commits into
mainfrom
issue/143-2

Conversation

@lhapp27

@lhapp27 lhapp27 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator

This should resolve issue #143

I had again troubles running the tests locally (see #148), so I do the PR without local tests. It may fail, let's see.

@ohno ohno left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

@lhapp27
The constructor functions can be eliminated by using @kwdef.
#143 (comment)

@lhapp27

lhapp27 commented Aug 5, 2026

Copy link
Copy Markdown
Collaborator Author

@lhapp27 The constructor functions can be eliminated by using @kwdef. #143 (comment)

But what about the default values, I assume we loose them? Is that a problem?

@ohno

ohno commented Aug 6, 2026

Copy link
Copy Markdown
Owner

@lhapp27
The default values ​​originally held by the struct were moved to an external constructor function in the PR 93092e8. I think we should revert this change because we eliminate the constructor functions. Please set the default values ​​as follows.

@kwdef struct HarmonicOscillator
k = 1.0
m = 1.0
= 1.0
end

@ohno ohno added this to the v1.0.0 milestone Aug 6, 2026
@ohno ohno added the joss Issues related to JOSS reviews label Aug 6, 2026
@codecov

codecov Bot commented Aug 6, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 84.64%. Comparing base (af8d9ba) to head (ee64a2a).

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #149      +/-   ##
==========================================
- Coverage   87.26%   84.64%   -2.63%     
==========================================
  Files          11       11              
  Lines         377      306      -71     
==========================================
- Hits          329      259      -70     
+ Misses         48       47       -1     

☔ View full report in Codecov by Harness.
📢 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.

@lhapp27

lhapp27 commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator Author

I had to add @kwdef to all modules separately. Not sure why this error did not show up in local tests. maybe it is related to #148 .
At least the current PR should be fine now.

@lhapp27
lhapp27 requested a review from ohno August 6, 2026 10:39
@ohno
ohno merged commit 96b9b21 into main Aug 6, 2026
6 checks passed

@ohno ohno left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thank you for contribution. I will close #143.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

joss Issues related to JOSS reviews

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants