Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix value_type and key_type for Tuple #59

Merged
merged 4 commits into from
Aug 26, 2024
Merged

Fix value_type and key_type for Tuple #59

merged 4 commits into from
Aug 26, 2024

Conversation

blegat
Copy link
Member

@blegat blegat commented Aug 21, 2024

No description provided.

Copy link

codecov bot commented Aug 21, 2024

Codecov Report

Attention: Patch coverage is 71.42857% with 6 lines in your changes missing coverage. Please review.

Project coverage is 85.93%. Comparing base (ba3697f) to head (7836f8e).
Report is 1 commits behind head on main.

Files Patch % Lines
src/coefficients.jl 40.00% 6 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main      #59      +/-   ##
==========================================
- Coverage   86.26%   85.93%   -0.33%     
==========================================
  Files          14       14              
  Lines         757      761       +4     
==========================================
+ Hits          653      654       +1     
- Misses        104      107       +3     
Flag Coverage Δ
unittests 85.93% <71.42%> (-0.33%) ⬇️

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.

kalmarek
kalmarek previously approved these changes Aug 21, 2024
@blegat
Copy link
Member Author

blegat commented Aug 22, 2024

@kalmarek Are you still ok now ?
For consistency with key_type, I changed everything to value_type. It also avoids having a code using valtype that works for everything except tuples as it forces people to use value_type otherwise it won't work for much.

@blegat
Copy link
Member Author

blegat commented Aug 22, 2024

We could also require using StaticArrays.SVector instead of tuples but the changes needed for tuples seem easy enough. SparseCoefficients are already weird enough so tuples aren't such freaks

@blegat blegat merged commit bbcd649 into main Aug 26, 2024
14 of 22 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