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

Add sparkline widget. #2631

Merged
merged 16 commits into from
Jun 1, 2023
Merged

Add sparkline widget. #2631

merged 16 commits into from
Jun 1, 2023

Conversation

rodrigogiraoserrao
Copy link
Contributor

@rodrigogiraoserrao rodrigogiraoserrao commented May 23, 2023

Implementation of the sparkline widget.

I'd like to validate the API before writing tests and docs.

I used $success and $error for the two default colours but their interpolation doesn't look great, IMO.
The example below is pretty much the example from the sparkline renderable file.

Untitled

@willmcgugan
Copy link
Collaborator

Agree re color. How about $accent as the peak color, and $accent 30% as the base?

Might need a little experimenting to find something that works.

Related comments: #2631
@rodrigogiraoserrao rodrigogiraoserrao linked an issue May 26, 2023 that may be closed by this pull request
@rodrigogiraoserrao
Copy link
Contributor Author

@willmcgugan some colour experiments here:

Untitled
App with experiments
from textual.app import App
from textual.widgets._sparkline import Sparkline

import statistics


class MyApp(App):
    CSS = """
    Sparkline {
        width: 100%;
        margin: 2 0;
    }
    #fst > .sparkline--max-color {
        color: $success;
    }
    #fst > .sparkline--min-color {
        color: $warning;
    }
    #snd > .sparkline--max-color {
        color: $warning;
    }
    #snd > .sparkline--min-color {
        color: $success;
    }
    #trd > .sparkline--max-color {
        color: $error;
    }
    #trd > .sparkline--min-color {
        color: $warning;
    }
    #frt > .sparkline--max-color {
        color: $warning;
    }
    #frt > .sparkline--min-color {
        color: $error;
    }
    #fft > .sparkline--max-color {
        color: $accent;
    }
    #fft > .sparkline--min-color {
        color: $accent 30%;
    }
    #sxt > .sparkline--max-color {
        color: $accent 30%;
    }
    #sxt > .sparkline--min-color {
        color: $accent;
    }
    #svt > .sparkline--max-color {
        color: $error;
    }
    #svt > .sparkline--min-color {
        color: $error 30%;
    }
    #egt > .sparkline--max-color {
        color: $error 30%;
    }
    #egt > .sparkline--min-color {
        color: $error;
    }
    #nnt > .sparkline--max-color {
        color: $success;
    }
    #nnt > .sparkline--min-color {
        color: $success 30%;
    }
    #tnt > .sparkline--max-color {
        color: $success 30%;
    }
    #tnt > .sparkline--min-color {
        color: $success;
    }
    """

    def compose(self):
        nums = [10, 2, 30, 60, 45, 20, 7, 8, 9, 10, 50, 13, 10, 6, 5, 4, 3, 7, 20]
        yield Sparkline(nums, summary_function=max, id="fst")
        yield Sparkline(nums, summary_function=max, id="snd")
        yield Sparkline(nums, summary_function=max, id="trd")
        yield Sparkline(nums, summary_function=max, id="frt")
        yield Sparkline(nums, summary_function=max, id="fft")
        yield Sparkline(nums, summary_function=max, id="sxt")
        yield Sparkline(nums, summary_function=max, id="svt")
        yield Sparkline(nums, summary_function=max, id="egt")
        yield Sparkline(nums, summary_function=max, id="nnt")
        yield Sparkline(nums, summary_function=max, id="tnt")


app = MyApp()
if __name__ == "__main__":
    app.run()

@rodrigogiraoserrao rodrigogiraoserrao changed the title Sparkline widget proof of concept. Add sparkline widget. May 29, 2023
@rodrigogiraoserrao rodrigogiraoserrao marked this pull request as ready for review May 29, 2023 16:42
@davep
Copy link
Contributor

davep commented May 30, 2023

Looks like roadmap should get a tweak in this PR too.

@davep
Copy link
Contributor

davep commented May 30, 2023

Looking over this, I wonder if the documentation for the widget needs fleshing out a little? I've never personally used a sparkline for anything, ever, so perhaps my ignorance of what they are and why I'd use them comes into play here. But the examples don't really give me a sense of why I'd use this, and to some degree I'm a little confused about the how too.

I can see that I feed it data, which will be numeric, but then it's not clear to me what the summary function actually does and why I'd want to use it. Also, there's talk of "buckets" but no explanation of what a bucket it is and how it comes into play here.

I feel that perhaps this widget deserves a good, simple, "real world" example that walks someone though the how and why of using the widget. Why would I use it? What sort of data is it good for? What is a bucket? What choices make sense for the summary function and how do I evaluate those choices?

src/textual/widgets/_sparkline.py Outdated Show resolved Hide resolved
src/textual/widgets/_sparkline.py Outdated Show resolved Hide resolved
@rodrigogiraoserrao
Copy link
Contributor Author

@willmcgugan @davep I've improved the docs but I still wasn't very verbose.
Let me know where you stand.

@davep
Copy link
Contributor

davep commented May 31, 2023

@rodrigogiraoserrao The expansion to the docs makes a lot more sense to be; I feel it's a lot more clear how it works and the role the functions play. I think the only thing I'd mention from these changes is that, rather than mention a "bucket" then go on to explain what it is in a tip, I think I'd start out in the main text talking about how the data gets portioned up to fit in the widget's width.

Maybe? Perhaps?

@willmcgugan
Copy link
Collaborator

I don't think the term "bucket" is relevant as far as the textual dev is concerned.

All they need to know is that they can assign some data, and it will be stretched to fit the space allocated to it.

docs/widgets/sparkline.md Outdated Show resolved Hide resolved
docs/widgets/sparkline.md Outdated Show resolved Hide resolved
src/textual/widgets/_sparkline.py Outdated Show resolved Hide resolved
rodrigogiraoserrao and others added 3 commits May 31, 2023 14:20
@rodrigogiraoserrao
Copy link
Contributor Author

@willmcgugan @davep I got rid of the word "bucket".

Copy link
Contributor

@davep davep left a comment

Choose a reason for hiding this comment

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

That last comment about the sparkline_basic.py example aside... looking good to me.

@willmcgugan willmcgugan merged commit 78db024 into main Jun 1, 2023
@willmcgugan willmcgugan deleted the sparkline branch June 1, 2023 08:34
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.

Sparkline widget
3 participants