Skip to content

Conversation

@KnutAM
Copy link
Member

@KnutAM KnutAM commented Jun 6, 2024

Solves type-stable construction for AbstractValues by introducing the singelton type ValuesUpdateFlags. In this PR, it is now also possible to have type-stable construction of CellValues, PointValues, and FacetValues with custom update specifications by providing kwargs as Val types. Using booleans still works and remain the documented way, but won't be type-stable.

Closes #960

Comment on lines +156 to +157
GradT = shape_gradient_type(fv)
sdim = GradT === nothing ? nothing : sdim_from_gradtype(GradT)
Copy link
Member Author

Choose a reason for hiding this comment

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

Fixes show of FacetValues with update_gradients=false

@codecov
Copy link

codecov bot commented Jun 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.70%. Comparing base (3ae61fa) to head (0067a78).

Additional details and impacted files
@@           Coverage Diff           @@
##           master     #979   +/-   ##
=======================================
  Coverage   93.69%   93.70%           
=======================================
  Files          36       36           
  Lines        5440     5445    +5     
=======================================
+ Hits         5097     5102    +5     
  Misses        343      343           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@KnutAM KnutAM marked this pull request as ready for review June 6, 2024 19:37
@KnutAM KnutAM requested a review from lijas June 6, 2024 20:23
@KnutAM KnutAM merged commit 1bab2f8 into master Jun 27, 2024
@KnutAM KnutAM deleted the kam/typestablefacetvalues branch June 27, 2024 08:03
@fredrikekre
Copy link
Member

Is the Val needed? I tried out this PR without them and it seemed to work anyway (I forgot to comment about it back when I tried).

@KnutAM
Copy link
Member Author

KnutAM commented Jun 27, 2024

The Val makes it possible to have it type-stable also when giving custom options (which is used internally). At least I couldn't get it type-stable without, but I deliberately made this internal as noted in the docstring, to allow us to remove this in the future.

Do you have an example where it works without?

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.

FacetValue does not seem to be type stable

3 participants