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

PSU summarizer in item view is split into rows weirdly #162

Closed
lvps opened this issue Nov 14, 2020 · 6 comments
Closed

PSU summarizer in item view is split into rows weirdly #162

lvps opened this issue Nov 14, 2020 · 6 comments

Comments

@lvps
Copy link
Member

lvps commented Nov 14, 2020

Update the summarizer to remove () so it doesn't look weird when split in multiple lines

@lvps
Copy link
Member Author

lvps commented Dec 4, 2020

Also cases and "socket 478, desktop"

@lvps
Copy link
Member Author

lvps commented Jan 31, 2021

Better but harder alternative: split on ( and ), but not on the commas between them

@Learath2 Learath2 self-assigned this Nov 23, 2022
Learath2 added a commit to Learath2/tarallo that referenced this issue Nov 23, 2022
@Learath2
Copy link
Collaborator

Better but harder alternative: split on ( and ), but not on the commas between them

This breaks Motherboard summaries, the only way to properly do this seems to be in returning an array of strings from Summarizers as only it knows the semantic components of a summary. I can do that if you don't mind large changes.

@lvps
Copy link
Member Author

lvps commented Nov 24, 2022

@Learath2 Yes please, do the array thing, I've been thinking about it for ages but never found the time to implement it. The new API that returns summaries should also return an array, too

@Learath2
Copy link
Collaborator

Q: What is our API policy? Can we break stuff or should we introduce a new endpoint for the new behaviour? A string -> array change will most definitely break some API clients (if there are any that use the summary endpoint that is).

@lvps
Copy link
Member Author

lvps commented Nov 24, 2022

Since all clients are under our control, we generally modify clients when we make big API changes. If that's not possible, we can make a new endpoint, but that was never necessary.

I can also tell you there are no clients at the moment using the summary endpoint, so go ahead and modify it.
Once that is done and the API is finalized, we can implement WEEE-Open/pytarallo#58 and part of WEEE-Open/weeelab-telegram-bot#25

Learath2 added a commit to Learath2/tarallo that referenced this issue Nov 25, 2022
Quadrollopo added a commit that referenced this issue Dec 15, 2022
Rework summarizer to output in tokens. Close #162
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants