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

[review] Fix font specific queries and new queries #3122

Merged
merged 2 commits into from
Sep 19, 2022

Conversation

bramstein
Copy link
Contributor

@bramstein bramstein commented Sep 12, 2022

Progress on #2883

This PR contains some fixes to font internals specific queries. The biggest change is that the font queries now no longer use pages, but instead only use requests.

The reason for that is that a popular font can be included in the requests table many times which would skew the results. Instead the queries that look at font-internal data now look at the unique font requests, which produces a more much accurate reflection of what internal font formats are being created by type designers.

This PR also contains a couple of new queries that were requested during the writing process or the technical review.

The data sheet and chapter has already been updated with the most recent data.

@rviscomi rviscomi added the analysis Querying the dataset label Sep 13, 2022
@rviscomi rviscomi added this to In Progress in 2022 via automation Sep 13, 2022
@rviscomi rviscomi added this to the 2022 Analysis milestone Sep 13, 2022
Copy link
Member

@rviscomi rviscomi left a comment

Choose a reason for hiding this comment

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

Queries look fine but a few of them are no longer grouping by client. This is required, otherwise we'd be double-counting requests on pages that exists for both client types, which skews the results. These queries would need to be updated and rerun, which will affect the results.

@bramstein
Copy link
Contributor Author

@rviscomi That is on purpose: for these queries I'm only interested in what is supported by unique font files. Wouldn't grouping by url achieve that?

@rviscomi
Copy link
Member

rviscomi commented Sep 13, 2022

Yeah, it's just for consistency across chapters that we require stats to be scoped to either desktop or mobile pages. Even if your metric is % of requests, they must be distinctly sourced from either/both desktop or mobile, but not combined. It's fine to pick one and report on it in the chapter (typically mobile), rather than sharing two similar stats. See the query conventions for more info.

@bramstein
Copy link
Contributor Author

@rviscomi: Thanks for the explanation! I've updated the queries that didn't include client. I've updated the sheet and chapter as well (there were minimal changes as the numbers only changed by a couple decimal points at most.)

Copy link
Member

@rviscomi rviscomi left a comment

Choose a reason for hiding this comment

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

Looks great, thanks for making the changes!

@rviscomi rviscomi merged commit 9c275cf into main Sep 19, 2022
2022 automation moved this from In Progress to Done Sep 19, 2022
@rviscomi rviscomi deleted the 2022-font-queries-update branch September 19, 2022 21:53
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analysis Querying the dataset
Projects
2022
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants