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

Remove SparseVarArray #31

Merged
merged 20 commits into from
Oct 24, 2022
Merged

Remove SparseVarArray #31

merged 20 commits into from
Oct 24, 2022

Conversation

hellemo
Copy link
Member

@hellemo hellemo commented Oct 19, 2022

No description provided.

@hellemo hellemo marked this pull request as draft October 19, 2022 18:27
@hellemo hellemo mentioned this pull request Oct 19, 2022
8 tasks
@codecov-commenter
Copy link

codecov-commenter commented Oct 19, 2022

Codecov Report

Base: 83.95% // Head: 84.98% // Increases project coverage by +1.03% 🎉

Coverage data is based on head (c4040b7) compared to base (1072681).
Patch coverage: 88.88% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main      #31      +/-   ##
==========================================
+ Coverage   83.95%   84.98%   +1.03%     
==========================================
  Files           6        4       -2     
  Lines         405      293     -112     
==========================================
- Hits          340      249      -91     
+ Misses         65       44      -21     
Impacted Files Coverage Δ
src/dictionaries.jl 85.48% <0.00%> (-2.55%) ⬇️
src/indexedarray.jl 90.19% <100.00%> (-0.96%) ⬇️
src/sparsearray.jl 81.25% <100.00%> (+27.08%) ⬆️
src/tables.jl 63.15% <100.00%> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hellemo hellemo marked this pull request as ready for review October 23, 2022 09:37
@hellemo hellemo requested a review from trulsf October 23, 2022 09:37
@trulsf
Copy link
Member

trulsf commented Oct 23, 2022

Nice cleanup! Consider adding a version of the Tables support without waiting for JuMP support, or will this be considered a breaking change if we move rowtable from SparseVariables to JuMP.Containers at a later stage?

@hellemo
Copy link
Member Author

hellemo commented Oct 24, 2022

Nice cleanup! Consider adding a version of the Tables support without waiting for JuMP support, or will this be considered a breaking change if we move rowtable from SparseVariables to JuMP.Containers at a later stage?

I guess I replied to the wrong comment just now. Yes, I think we can do this as long as we don't export rowtable and make it clear we don't support it until it's added in an new JuMP release.

@hellemo hellemo merged commit 7b85efa into main Oct 24, 2022
@hellemo hellemo deleted the lh/remove-sparsevararray branch October 24, 2022 15:53
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.

3 participants