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

feat: dx.indicator spec #1062

Merged
merged 8 commits into from
Dec 20, 2024
Merged

Conversation

jnumainville
Copy link
Collaborator

@jnumainville jnumainville commented Dec 13, 2024

spec for #1019

I haven't really written a spec for dx before but it made sense to me to just put it in a new function with NotImplementedError.

Instead of just enabling one indicator, you can create multiple at one time with a plot by that makes subplots of indicators. This enables, for example, the creation of a new indicator automatically as a new symbol ticks in.
The default behavior is only a number if you only provide value and number+delta if you provide reference as well.
https://plotly.com/python/indicator/

Syntax is otherwise generally in line with other dx functions and most other behavior is fairly straightforward.

Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Makes sense to me... only thing I'd like is to be able to pass in a value that I have outside of a table as well. Calculated from a user input or something.
It could be that we pump it through a table anyway, but I'd like to see an example of how to hook that up.

@jnumainville
Copy link
Collaborator Author

jnumainville commented Dec 17, 2024

Makes sense to me... only thing I'd like is to be able to pass in a value that I have outside of a table as well. Calculated from a user input or something. It could be that we pump it through a table anyway, but I'd like to see an example of how to hook that up.

Added an example of putting some values in a table.
I think if we want something more comprehensive, I'd argue it's beyond the scope of this initial indicator implementation because what we would really want to do is rework what we allow passed into table for all plots. Things like dicts, lists, etc. I don't really like the idea of breaking the abstraction by passing in values directly to parameters that take columns (like value and reference) as it adds complexity and inconsistency, but I'd be open to expanding what we allow passed into table.

mofojed
mofojed previously approved these changes Dec 18, 2024
- **Highlight specific metrics**: Indicator plots are useful when you want to highlight specific numeric metrics in a visually appealing way.
- **Compare metrics to a reference value**: Indicator plots are useful to compare metrics to a reference value, such as a starting value or a target value.
- **Compare metrics to each other**: Indicator plots are useful to compare multiple metrics to each other by highlighting where they fall relative to each other.
-
Copy link
Contributor

Choose a reason for hiding this comment

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

stray dash

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

removed

Copy link
Member

@mofojed mofojed left a comment

Choose a reason for hiding this comment

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

Looks good to me.

This has inspired me to create a utility function called dict_table for where you just want a table from a dictionary, e.g.

from deephaven import dict_table

my_table = dict_table({
  "MyValue": [my_value],
  "MyReference": [my_reference],
})

Or even just assume if it's not a list passed in, that it is just a one row table and you don't need to pass in an array explicitly, e.g.:

from deephaven import dict_table

my_table = dict_table({
  "MyValue": my_value,
  "MyReference": my_reference,
})

Don't have as control over types ... but it would be nice for creating quick tables like in this case.

@jnumainville jnumainville merged commit 4478013 into deephaven:main Dec 20, 2024
18 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.

4 participants