Add more column options to p-adic field dynamic statistics - #27
Add more column options to p-adic field dynamic statistics#27roed-math wants to merge 5 commits into
Conversation
Extend LFStats.dynamic_cols from just galois_label/slopes to every
searchable column (p, n, e, f, c, galois_label, aut, u, t, top_slope,
slopes, visible, hidden, ind_of_insep, associated_inertia, jump_set), so
users can build their own statistics and click through to matching
searches. Supporting display metadata (short_display, top_titles,
knowls, formatters, sort_keys, query_formatters, default p/c buckets) is
added to match the search boxes. sort_keys map None to a first-sorting
sentinel so columns with NULLs (slopes, hidden, u, t, galois_label) no
longer crash when picked unconstrained; query_formatters emit valid
search URLs (topslope=, slopes=...&slopes_quantifier=exactly, empty
constraints for un-searchable values). galformatter/galsortkey/galquery
now tolerate the 3,996 NULL galois_labels. discdisp pins buckets={} so
the static /stats page is unchanged. Links to the dynamic stats page are
added from the browse index (short_summary) and /stats (summary).
Verified: sage -python -m pytest lmfdb/local_fields/test_localfields.py
(10 passed, incl. new test_stats_pages/test_dynamic_stats); /padicField/stats
byte-identical to base except the added "create your own" link (fixed
PYTHONHASHSEED diff); formatter/sort-key/query-formatter sweep over all
distinct DB values of the new columns with no errors; new-style
click-through URLs resolve (e.g. slopes exact match returns 10 fields);
pyflakes clean.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…mic stats (LMFDB#6542) P2: the dynamic-statistics query_formatters encoded not-computed/null buckets as empty url parameters (gal=, u=, slopes=, associated_inertia=), which the search parsers silently ignore -- so clicking a null bucket's count opened an UNFILTERED search returning every field instead of the counted records. There is no url-level null-search syntax (empty params are dropped; $exists:False is not reachable from any parser used by common_parse), so the drill-down link is now suppressed for such buckets. The null query_formatters return a new NO_SEARCH_QUERY sentinel; LFStats.display_data (via the testable _suppress_null_links helper) blanks any assembled drill-down url containing it, and stat_1d.html now guards the link with {% if c.query %} exactly as stat_2d.html already did. P3: formatbracketcol handled empty lists/strings but not None, so ind_of_insep/ associated_inertia could render a literal $None$ bucket label; None now maps to the standard "not computed" presentation. Verified (sage -python, devmirror): pytest lmfdb/local_fields/test_localfields.py -> 12 passed, incl. two new tests (unit coverage of the sentinel query_formatters, _suppress_null_links for counts+grid, and formatbracketcol(None/''/[]); page-level coverage that the galois_label/slopes/hidden not-computed buckets render with no empty-parameter link and no sentinel leak, while non-null buckets still filter correctly). Confirmed the not-computed galois_label bucket (3996 fields) now has a blank query, and non-null drill-downs (p=2,n=4 -> 12 cyclic quartics; n x e) still produce correct filtered links. pyflakes clean. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the external review's P2 and P3 findings: commit d120ea2. P2 (null-bucket drill-downs): confirmed the parsers silently drop empty params and no null-search URL syntax exists, so suppression is the right policy. Null branches of the query formatters now return a NO_SEARCH_QUERY sentinel, LFStats blanks any drill-down containing it (via a testable helper), and stat_1d.html guards the count link the same way stat_2d.html already did. Concrete impact: the cached not-computed galois_label bucket (3,996 fields) previously linked to an unfiltered ~1.39M-row search; it now renders linkless. P3: formatbracketcol(None) renders "not computed" instead of 12 local_fields tests pass (2 new); non-null drill-downs verified to still filter correctly; stayed clear of PR #26's hunks in the same file. |
Three ways a count on the dynamic statistics page could link to a search returning different records than the count, all of them in the generic statistics framework rather than the p-adic tables: Cells of a two-dimensional grid with no records were synthesized by KeyedDefaultDict from the formatted row and column headers, so their urls were built from displayed values (TeX, html, "not computed") rather than stored ones. The totaler intersects the urls of a row to find the row's constraint, so a sparse row lost it: the "not nilpotent" total on the abstract groups statistics page linked to /Groups/Abstract/? and returned all 1.5 million groups instead of the 455903 counted. display_data now indexes counts by the stored values and builds every url, empty cells included, from those, so a formatter is free to produce TeX or html. Values that display identically share a row, adding their counts, rather than one silently replacing the other. Drill-down urls were serialized from the parsed query, whose columns are often not parameters the search page accepts: dynamic statistics constrained by Artin slopes produced slopes_tmp=2A, which the p-adic parser ignores, so clicking a count opened a broader search. dynamic_setup now passes the search boxes the user filled in as link_constraint, which reproduce the same query by construction, and a test asserts they re-parse to it. Bucket endpoints for top_slope were compared as text against the fixed-width decimal encoding the column is stored in, so a bucket of 1-2 counted the wrong fields, and topslope_query sliced 12 characters off every endpoint, turning the bucket 1-2 into topslope=-. Buckets are now encoded through a bucket_encoders hook before they reach the backend and decoded for display and links, and an endpoint that is not a rational is rejected with the usual message. Along the way: the sentinel for unsearchable values moves into the framework, so a null bucket is linkless in every table rather than only in p-adic ones; totals that share no constraint are no longer linked; total urls list their parameters in a fixed order rather than set order; invalid input to the dynamic statistics page flashes an error instead of raising a 500; the default p and c buckets are left open above so no field is omitted as the database grows; a not-computed jump set is distinguished from an empty one; and nilp_qformatter maps the stored -1 to nilpotent=no, since nilpotency_class=-1 matches nothing. Every statistics page in LMFDB renders identically to before apart from those fixes. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Addressed the three drill-down/bucketing problems from the review, in 1. Sparse two-dimensional grids build urls from stored values
This was not hypothetical, and not limited to p-adic fields. On the abstract groups statistics page, the "not nilpotent" row totalled 455903 groups and linked to
Two related repairs fell out: a row whose cells share no constraint is no longer linked at all (the guard existed for column totals but not row totals), and 2. Drill-downs use the search boxes, not the parsed query
stats.dynamic_parse(info, query)
link = stats.dynamic_link_constraint(info, ['n'])
common_parse(to_dict(dict(parse_qsl(link))), reparsed)
assert reparsed == query # and '_tmp' not in link, '=None' not in linkColumns being displayed are excluded via a new 3. Top-slope buckets are encoded before comparison
There is now a assert self._url_count('/padicField/?topslope=1-2') == db.lf_fields.count(
{'top_slope': {'$gte': ratproc('1'), '$lte': ratproc('2')}}) # 25620A malformed endpoint is now rejected with the usual message instead of an empty table. Also
Checks
One thing worth knowing when reviewing: the devmirror is read only, so statistics for combinations that are not already cached come back empty and their tables render blank. That is why the count-comparison tests are written against combinations that are cached ( |
…s it The total row appended the bare column name to the url as a marker, which is not a search constraint: an empty parameter is ignored by the parsers, so the link returned every record rather than the ones counted, and the parameter is often not even one the search page accepts (galois_label rather than gal). The statistics backend computes a total over the records where the column is not null, so the discrepancy is real wherever a column is not computed for every record. On the classical modular forms statistics page the analytic rank block totalled the 281885 forms whose analytic rank is known and linked to a search returning all 1141510 newforms; dynamic statistics for Galois groups with p=7 and n=21 totalled 1324 of 3784 fields. A total is now linked only when the search page can describe exactly the records it counts: the constraint itself, which is right when the column is computed for every record satisfying it. Totals over buckets (which cover only the buckets displayed), over split lists (which count entries rather than records), and over a column the constraint also restricts (whose parameters the urls leave out, so that each count can constrain it) are rendered without a link. Also fixes the test helper that reads a one-dimensional table off the page: it matched nothing, so two page-level loops were passing vacuously. With it working, the pairing of an addl_row_title row with its values turns out to have been repaired too, by the same change that made the headers carry their own values: on the genus 2 statistics page the real geometric endomorphism algebras were rotated against the counts underneath them, labelling the 6 curves with endomorphism algebra C x C as M_2(R). Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
Fixed in The Total row
The gap is real wherever a column is not computed for every record, because the statistics backend totals over the non-null ones (
A total is now linked only when the search page can describe exactly the records it counts, which is the constraint itself, and only when the total covers every record satisfying it: if buckets or split_list or any(col in constraint for col in cols):
return ''
if total != table.table.count(constraint):
return ''
return base_url + '&'.join(extras)Buckets and split lists are excluded for the reasons the brief gives. The third clause is a case the brief does not mention: when the constraint restricts a column that is also being displayed, the urls deliberately leave that column out (each count constrains it itself), so the total would claim more records than it counted. Otherwise the exactness test is a direct comparison against the number of records the constraint matches, rather than against the sum of the displayed counts, since a null row is only displayed when the cached statistics happen to include one. Where the column is computed for everything, the link survives and is now honest: Tests
One thing the follow-up did not ask aboutWriting those tests turned up a defect in my own test helper: With the helper working, the page comparison I reported last time turned out to have the same blind spot: it only captured
Everything else across the site is byte-identical to the PR base once url parameter order is normalized, which the previous round changed from set order to a fixed order. Checks
The extra |
|
GPT signed off. |
1 similar comment
|
GPT signed off. |
|
Superseded by LMFDB#7163, opened upstream from this same branch. Closing here; review continues upstream. |
The dynamic statistics page for p-adic fields previously offered only Galois
group and Galois Artin slopes as columns. This extends
LFStats.dynamic_colstoevery searchable column (residue characteristic, degree, ramification index,
residue field degree, discriminant exponent, Galois group, automorphisms,
unramified/tame degree, top slope, Galois/visible/hidden slopes, indices of
inseparability, associated inertia, jump set), with display metadata and
click-through query formatters matched to the existing search boxes. Sort keys
map not-computed (NULL) values to sort first so columns with NULLs no longer
crash when selected unconstrained, and Galois-group handling tolerates fields
with no computed Galois group. Default
p/cbuckets are provided, and thestatic statistics page is verified unchanged (discdisp pins empty buckets).
Links to "create your own" statistics are added from the browse index and the
statistics page. Adds tests covering the links and rendering of all new columns.
Addresses LMFDB#6542.
🤖 Generated with Claude Code