-
Notifications
You must be signed in to change notification settings - Fork 62
docs: adding translation stats to docs #511
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?
Changes from all commits
e018c21
f2ce21d
a3567ca
3dfe760
b3e26d8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||
---|---|---|---|---|---|---|---|---|---|---|
@@ -0,0 +1,85 @@ | ||||||||||
from pathlib import Path | ||||||||||
import json | ||||||||||
from typing import TypeAlias, TypedDict, Annotated as A | ||||||||||
|
||||||||||
from docutils import nodes | ||||||||||
from docutils.parsers.rst import Directive | ||||||||||
import plotly.graph_objects as go | ||||||||||
from plotly.offline import plot | ||||||||||
|
||||||||||
Comment on lines
+8
to
+9
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Suggested change
oops, forgot that i added this (though this is just trying to follow the docs where it says it must be an array, but i bet it would work fine just as a list of lists) |
||||||||||
|
||||||||||
class ModuleStats(TypedDict): | ||||||||||
total: int | ||||||||||
translated: int | ||||||||||
fuzzy: int | ||||||||||
untranslated: int | ||||||||||
percentage: float | ||||||||||
|
||||||||||
TranslationStats: TypeAlias = dict[A[str, "locale"], dict[A[str, "module"], ModuleStats]] | ||||||||||
|
||||||||||
|
||||||||||
class TranslationGraph(Directive): | ||||||||||
# Tells Sphinx that this directive can be used in the document body | ||||||||||
# and has no content | ||||||||||
has_content = False | ||||||||||
|
||||||||||
def run(self): | ||||||||||
# Read the JSON file containing translation statistics | ||||||||||
json_path = Path(__file__).parent.parent / "_static" / "translation_stats.json" | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. i missed the PR that added the I personally avoid committing generated data files that only need to exist at deployment time because they make PRs noisy and tempt us to treat them as files we can edit, but if we are to keep it there, we should add it and trigger it from some early build event like There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hi @sneakers-the-rat! Yeah that makes sense - I could move the generation of the JSON file into a build_event. That sounds reasonable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. it took me many years and many attempts of trying before the hook pattern of sphinx sunk in for me, the way you did it was completely understandable |
||||||||||
with json_path.open("r") as f: | ||||||||||
data: TranslationStats = json.load(f) | ||||||||||
|
||||||||||
# Collect all module names -- iterates over the JSON data in 2 levels | ||||||||||
all_modules = {module for stats in data.values() for module in stats} | ||||||||||
all_modules = sorted(all_modules) | ||||||||||
|
||||||||||
# Build one trace per locale with full hover info | ||||||||||
traces = [] | ||||||||||
|
||||||||||
for locale, modules in data.items(): | ||||||||||
y_vals = [] | ||||||||||
hover_texts = [] | ||||||||||
|
||||||||||
for module in all_modules: | ||||||||||
stats = modules.get(module) | ||||||||||
y_vals.append(stats["percentage"]) | ||||||||||
|
||||||||||
hover_text = ( | ||||||||||
f"<b>{module}</b><br>" | ||||||||||
f"Translated: {stats['translated']}<br>" | ||||||||||
f"Fuzzy: {stats['fuzzy']}<br>" | ||||||||||
f"Untranslated: {stats['untranslated']}<br>" | ||||||||||
f"Total: {stats['total']}<br>" | ||||||||||
f"Completed: {stats['percentage']}%" | ||||||||||
) | ||||||||||
hover_texts.append(hover_text) | ||||||||||
|
||||||||||
traces.append(go.Bar( | ||||||||||
name=locale, | ||||||||||
x=all_modules, | ||||||||||
y=y_vals, | ||||||||||
hovertext=hover_texts, | ||||||||||
hoverinfo="text" | ||||||||||
)) | ||||||||||
|
||||||||||
# Create figure | ||||||||||
fig = go.Figure(data=traces) | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @RobPasMue could we create a grid of plots - one for each language? then each plot could have 3 bars - one for fuzzy, one for complete and one for incomplete (or it could be stacked bars too. What you have now is awesome but if we add more languages it will get complex over time. And a static version of the plot would be nice too. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure sounds good! There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think a good idea would be a heat map, which is condensed enough we can add many more languages. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. agreed on the heatmap. i would expect it oriented with languages as rows and pages as columns (which satisfies the need to expandability to future languages) |
||||||||||
fig.update_layout( | ||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could the plot please use our pyOS colors? Dark Purple: #33205c There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Most def! =) I'll look into the colors as soon as I can There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh whoops. just saw this comment. probably would be good to use the css variables directly when we can to avoid having them hardcoded in multiple places. i couldn't find a rhyme or reason to when i was able to use css vars in the plotly values and when i needed to declare them in the stylesheet, but ya some examples in this comment |
||||||||||
barmode="group", | ||||||||||
title="Translation Coverage by Module and Locale", | ||||||||||
xaxis_title="Module", | ||||||||||
yaxis_title="Percentage Translated", | ||||||||||
height=600, | ||||||||||
margin=dict(l=40, r=40, t=40, b=40) | ||||||||||
) | ||||||||||
|
||||||||||
div = plot(fig, output_type="div", include_plotlyjs=True) | ||||||||||
return [nodes.raw("", div, format="html")] | ||||||||||
|
||||||||||
def setup(app): | ||||||||||
app.add_directive("translation-graph", TranslationGraph) | ||||||||||
return { | ||||||||||
"version": "0.1", | ||||||||||
"parallel_read_safe": True, | ||||||||||
"parallel_write_safe": True, | ||||||||||
} |
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.
Maybe here would be a good spot to include the link to the site
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.
seems like we would want both directions? from the contributing page here and vice versa?