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

Fleshed out MetricsFrames and their various types to enforce better #468

Closed
wants to merge 4 commits into from

Conversation

mattieruth
Copy link
Contributor

consistency across processors and their different reports.

consistency across processors and their different reports.
@@ -55,7 +56,8 @@ def __post_init__(self):

def __str__(self):
pts = format_pts(self.pts)
return f"{self.name}(pts: {pts}, size: {len(self.audio)}, frames: {self.num_frames}, sample_rate: {self.sample_rate}, channels: {self.num_channels})"
return f"{self.name}(pts: {pts}, size: {len(self.audio)}, frames: {self.num_frames}, sample_rate: {
self.sample_rate}, channels: {self.num_channels})"
Copy link
Contributor

Choose a reason for hiding this comment

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

I had the same changes by autopep8. I think they broke something. If we do this change the output will be messed up with carriage return. What I do, before committing, is just revert only these changes. Very annoying, but I haven't found a better way.

Copy link
Contributor Author

@mattieruth mattieruth Sep 18, 2024

Choose a reason for hiding this comment

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

this one actually seems right to me (or almost right. this is wrapping at 108 😕 ). shouldn't it wrap at 100 chars? meanwhile, I agree.. something is up with the formatter because it seems inconsistent

@@ -0,0 +1,34 @@
from typing import Optional
Copy link
Contributor

Choose a reason for hiding this comment

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

Since this is a new module, we need an empty __init__.py


class TTFBMetricsData(MetricsData):
value: float
model: Optional[str]
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe we can initialize this to None so we don't need to pass it.

    model: Optional[str] = None


class ProcessingMetricsData(MetricsData):
value: float
model: Optional[str]
Copy link
Contributor

Choose a reason for hiding this comment

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

Same


class TTSUsageMetricsData(MetricsData):
processor: str
model: Optional[str]
Copy link
Contributor

Choose a reason for hiding this comment

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

    processor: str
    value: int
    model: Optional[str] = None

@@ -11,6 +11,7 @@
from abc import abstractmethod
from typing import AsyncGenerator, List, Optional, Tuple

from attr import has
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note to self: remove (not sure why this got added)

@mattieruth
Copy link
Contributor Author

closing in leu of #474

@mattieruth mattieruth closed this Sep 19, 2024
@aconchillo aconchillo deleted the ruthless/improve-metrics-types branch October 23, 2024 20:55
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.

2 participants