-
-
Notifications
You must be signed in to change notification settings - Fork 80
Add LiteralRef wrapper to assign ref-like values without resolving #1091
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
base: main
Are you sure you want to change the base?
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1091 +/- ##
==========================================
+ Coverage 89.15% 89.22% +0.07%
==========================================
Files 9 9
Lines 4685 4726 +41
==========================================
+ Hits 4177 4217 +40
- Misses 508 509 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
This looks like a useful functionality for advanced users and libraries that integrate Param (like Panel, Lumen, etc.). A few comments:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Somehow I missed until now that the wrapper was just transient. I think I stopped at "without resolving". I'm okay with that but was just wondering, @philippjfr is unwrapping more convenient than keeping it as-is? Unless you already know the Parameter is holding a literal ref, I guess you need a is_ref function? While if it's still wrapped as a LiteralRef, it's just an isinstance call?
| Example | ||
| ------- |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Example | |
| ------- | |
| Examples | |
| -------- |
| "outputs": [], | ||
| "source": [ | ||
| "class Example(param.Parameterized):\n", | ||
| " value = param.Parameter(allow_refs=True)\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just to avoid the possible confusion that this would only work with the base Parameter.
| " value = param.Parameter(allow_refs=True)\n", | |
| " value = param.String(allow_refs=True)\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well this will error, because the ref value (i.e. the Parameter instance) is not a string.
| "id": "481223e8-750e-41b4-8376-63381b2012c6", | ||
| "metadata": {}, | ||
| "source": [ | ||
| "#### Key points\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "#### Key points\n", | |
| "### Key points\n", |
| "- The wrapper is transient: it's unwrapped at assignment time; the stored value is the inner ref object.\n", | ||
| "- No subscription, no evaluation, no resolution occurs during assignment.\n", | ||
| "\n", | ||
| "#### When to use LiteralRef\n", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| "#### When to use LiteralRef\n", | |
| "### When to use LiteralRef\n", |
|
|
|
Can you also add |
rawwrapper to assign ref-like values without resolving #1090