diff --git a/lmfdb/groups/abstract/main.py b/lmfdb/groups/abstract/main.py index df7050d750..e5d391990e 100644 --- a/lmfdb/groups/abstract/main.py +++ b/lmfdb/groups/abstract/main.py @@ -1646,13 +1646,26 @@ def group_parse(info, query): parse_family(info, query, "family", qfield="label") parse_hashes(info, query, "hash", order_field="order") +def name_download_desc(what, extra=""): + """ + The download description for a column giving the name of the subgroup, ambient group or + quotient. These columns are displayed as a single name, but download as the pair of + database columns holding the LMFDB label and the TeX name, so we spell that pair out (#6477). + """ + return (f"A two-element list [label, name] for the {what} as an abstract group: its LMFDB label, " + "followed by its name formatted in TeX (see the group.name knowl for the conventions " + "used in these names).\n" + "The label is null when the abstract group is not in the LMFDB; the name is still given " + f"in that case.{extra}") + subgroup_columns = SearchColumns([ LinkCol("label", "group.subgroup_label", "Label", get_sub_url, th_class=" border-right", td_class=" border-right"), ColGroup("subgroup_cols", None, "Subgroup", [ MultiProcessedCol("sub_name", "group.name", "Name", ["subgroup", "subgroup_tex"], display_url, - short_title="Sub. name", apply_download=False), + short_title="Sub. name", apply_download=False, + download_desc=name_download_desc("subgroup")), ProcessedCol("subgroup_order", "group.order", "Order", show_factor, align="center", short_title="Sub. order"), ProcessedCol("sylow", "group.sylow_subgroup", "Sylow", lambda x: f"${latex(x)}$" if x > 1 else "", align="center", short_title="Sub. Sylow"), CheckCol("normal", "group.subgroup.normal", "norm", short_title="Sub. normal"), @@ -1676,14 +1689,19 @@ def group_parse(info, query): MultiProcessedCol("ambient_name", "group.name", "Name", ["ambient", "ambient_tex"], display_url, - short_title="Ambient name", apply_download=False), + short_title="Ambient name", apply_download=False, + download_desc=name_download_desc("ambient group")), ProcessedCol("ambient_order", "group.order", "Order", show_factor, align="center", short_title="Ambient order")]), SpacerCol("", th_class=" border-right", td_class=" border-right", td_style="padding:0px;", th_style="padding:0px;"), ColGroup("quotient_cols", None, "Quotient", [ MultiProcessedCol("quotient_name", "group.name", "Name", ["quotient", "quotient_tex"], display_url, - short_title="Quo. name", apply_download=False), + short_title="Quo. name", apply_download=False, + download_desc=name_download_desc( + "quotient", + extra="\nBoth entries are null when the subgroup is not normal, " + "since then the quotient is not defined.")), ProcessedCol("quotient_order", "group.quotient_size", "Size", lambda n: show_factor(n) if n else "", align="center", short_title="Quo. size"), CheckCol("minimal_normal", "group.maximal_quotient", "max", short_title="Quo. maximal"), #next columns are None if non-normal so we set unknown to "-" instead of "?" diff --git a/lmfdb/groups/abstract/test_abstract_groups.py b/lmfdb/groups/abstract/test_abstract_groups.py index 4d6faea241..59b7ba6706 100644 --- a/lmfdb/groups/abstract/test_abstract_groups.py +++ b/lmfdb/groups/abstract/test_abstract_groups.py @@ -1,5 +1,59 @@ +import csv +import io +import re + from lmfdb.tests import LmfdbTest +# A search for the subgroups of a single small group (D_4), used to test the download of +# grouped columns. The query is the url encoding of {'ambient': '8.3'}. +SUBGROUP_DOWNLOAD = ("/Groups/Abstract/Subgroups?download=1" + "&query=%7B%27ambient%27%3A+%278.3%27%7D" + "&ambient=8.3&download_row_count=3&Submit=") + + +def list_entries(field): + """ + The top level entries of a list in a download file, as strings. Entries of nested + lists are not broken out, so this gives the length of the list as downloaded. + """ + assert field.startswith("[") and field.endswith("]"), field + entries = [''] + escaped = in_string = False + depth = 0 + for c in field[1:-1]: + if escaped: + escaped = False + elif in_string: + escaped = c == "\\" + in_string = c != '"' + elif c == '"': + in_string = True + elif c in "[(": + depth += 1 + elif c in ")]": + depth -= 1 + elif c == "," and depth == 0: + entries.append('') + continue + entries[-1] += c + return [entry.strip() for entry in entries] + + +def grouped_schema(page): + """ + The expansions of the grouped columns, as (title, [subcolumn titles]) pairs in order, + taken from the header comment of a text download. + """ + header = page.split("where the grouped columns are themselves lists:")[-1] + header = header.split("For more details")[0] + schema = [] + for line in header.split("\n"): + match = re.match(r"^#\s+(.+) = \[(.+)\]$", line) + if match: + schema.append((match.group(1), match.group(2).split(", "))) + return schema + + class AbGpsTest(LmfdbTest): # All tests should pass @@ -35,6 +89,121 @@ def test_abstract_group_download(self): self.assertTrue("monomial := true," in response.get_data(as_text=True)) self.assertTrue("CR := CharacterRing(G);" in response.get_data(as_text=True)) + def test_subgroup_search_download_text(self): + r""" + The subgroup columns are grouped, and each group downloads as a list of its + subcolumns' values, so the download must describe those subcolumns (#6477). + """ + page = self.tc.get(SUBGROUP_DOWNLOAD + "text").get_data(as_text=True) + + # The top level shape of a row is unchanged + self.assertIn("[Label, Subgroup, Ambient, Quotient]", page) + + # and each grouped column is expanded into its subcolumns, in order. We check the + # first and last subcolumn along with a few in between, so that an expansion that is + # truncated or reordered fails even if a column is added later. + schema = grouped_schema(page) + self.assertEqual([title for title, subs in schema], ["Subgroup", "Ambient", "Quotient"]) + expansions = dict(schema) + for title, expected in [ + ("Subgroup", ["Sub. name", "Sub. order", "Sub. normal", "Sub. central", "Sub. metacyclic"]), + ("Ambient", ["Ambient name", "Ambient order"]), + ("Quotient", ["Quo. name", "Quo. size", "Quo. abelian", "Quo. metabelian"]), + ]: + subs = expansions[title] + self.assertEqual([sub for sub in subs if sub in expected], expected) + self.assertEqual(subs[0], expected[0]) + self.assertEqual(subs[-1], expected[-1]) + + # The definitions at the bottom introduce the group and describe its subcolumns + self.assertIn(f" {title} is a grouped column, downloaded as a list of the following subcolumns:", page) + for title, name in [("Sub. name", "sub_name"), ("Sub. order", "subgroup_order"), + ("Sub. metacyclic", "metacyclic"), ("Ambient name", "ambient_name"), + ("Ambient order", "ambient_order"), ("Quo. name", "quotient_name"), + ("Quo. metabelian", "quotient_metabelian")]: + self.assertIn(f"{title} ({name}) --", page) + + # The name of a group downloads as a [label, TeX name] pair rather than a single name, + # which is documented for the subgroup, the ambient group and the quotient + for what in ["subgroup", "ambient group", "quotient"]: + self.assertIn(f"A two-element list [label, name] for the {what} as an abstract group", page) + + # Finally, the downloaded rows have the documented shape + rows = [line for line in page.split("\n") if line.startswith('"8.3.')] + self.assertTrue(rows) + for row in rows: + fields = row.split("\t") + self.assertEqual(len(fields), 4) + for field, title in zip(fields[1:], ["Subgroup", "Ambient", "Quotient"]): + entries = list_entries(field) + self.assertEqual(len(entries), len(expansions[title])) + # whose first entry is the [label, TeX name] pair + self.assertEqual(len(list_entries(entries[0])), 2) + + def test_subgroup_search_download_csv(self): + r""" + CSV files have no comments, so the header row itself has to describe the contents of + each grouped column (#6477). We keep one field per column, so the shape is unchanged. + """ + page = self.tc.get(SUBGROUP_DOWNLOAD + "csv").get_data(as_text=True) + rows = [row for row in csv.reader(io.StringIO(page)) if row] + header, data = rows[0], rows[1:] + self.assertEqual(len(header), 4) + + # An ordinary column is still just its name, linked to its knowl + self.assertTrue(header[0].startswith("=HYPERLINK("), header[0]) + self.assertTrue(header[0].endswith('"label")'), header[0]) + + # while a grouped column also lists the contents of its nested list, in order + for cell, name, expected in [ + (header[1], "subgroup_cols", ["sub_name", "subgroup_order", "normal", "metacyclic"]), + (header[2], "ambient_cols", ["ambient_name", "ambient_order"]), + (header[3], "quotient_cols", ["quotient_name", "quotient_order", "quotient_metabelian"]), + ]: + match = re.match(r"^(\w+) \[(.+)\]$", cell) + self.assertTrue(match, cell) + self.assertEqual(match.group(1), name) + subcols = match.group(2).split(", ") + self.assertEqual([sub for sub in subcols if sub in expected], expected) + self.assertEqual(subcols[0], expected[0]) + self.assertEqual(subcols[-1], expected[-1]) + + # Each row still has one field per top level column + self.assertTrue(data) + for row in data: + self.assertEqual(len(row), len(header)) + + def test_scalar_colgroup_download(self): + r""" + Column groups that set a download_col download as a single value rather than as a + list over their subcolumns, so they must not be expanded into subcolumns (#6477). + """ + from lmfdb.classical_modular_forms.main import newform_columns + from lmfdb.groups.abstract.main import conjugacy_class_columns, subgroup_columns + from lmfdb.utils.search_columns import ColGroup + + def get_col(columns, name): + # Some names are shared with a spacer column, so we ask for the group by type + return next(col for col in columns.columns + if col.name == name and isinstance(col, ColGroup)) + + for columns, name, download_col in [ + (newform_columns, "traces", "trace_display"), + (newform_columns, "atkin_lehner", "atkin_lehner_eigenvals"), + (conjugacy_class_columns, "power_cols", "powers"), + ]: + col = get_col(columns, name) + self.assertEqual(col.download_col, download_col) + self.assertEqual(col.download_subcols({}), []) + # and the value downloaded is the single stored column + self.assertEqual(col.download({download_col: "unchanged"}), "unchanged") + + # By contrast the subgroup groups, which have no download_col, are expanded + for name in ["subgroup_cols", "ambient_cols", "quotient_cols"]: + col = get_col(subgroup_columns, name) + self.assertIsNone(col.download_col) + self.assertEqual(col.download_subcols({}), col.subcols) + def test_conj_decode(self): from lmfdb.groups.abstract.web_groups import WebAbstractGroup G = WebAbstractGroup("18.2") diff --git a/lmfdb/tests/test_utils.py b/lmfdb/tests/test_utils.py index 3935d9e249..22e0624a0d 100644 --- a/lmfdb/tests/test_utils.py +++ b/lmfdb/tests/test_utils.py @@ -38,6 +38,12 @@ infinity, ) +from lmfdb.utils.search_columns import ( + ColGroup, + MathCol, + SearchCol, +) + class UtilsTest(unittest.TestCase): """ An example of unit tests that are not based on the website itself. @@ -207,6 +213,35 @@ def test_list_to_latex_matrix(self): malform_rep = '\\left(\\begin{array}{rr}1 & 0\\\\0\\end{array}\\right)' self.assertEqual(list_to_latex_matrix(malformed), malform_rep) + def test_download_subcols(self): + r""" + Checking utility: download_subcols, which identifies the columns that download as a + nested list of their subcolumns' values so that the downloader can document each + subcolumn (#6477). Columns downloading a single value must report no subcolumns, + since expanding them would describe data that isn't there. + """ + subcols = [MathCol("a", "test.a", "A"), MathCol("b", "test.b", "B")] + + # An ordinary column downloads a single value + self.assertEqual(SearchCol("plain", "test.plain", "Plain").download_subcols({}), []) + + # A group with no download_col downloads the list of its subcolumns' values, in order + group = ColGroup("group", None, "Group", subcols) + self.assertEqual(group.download_subcols({}), subcols) + self.assertEqual(group.download({"a": 1, "b": 2}), [1, 2]) + + # while a group with a download_col downloads that single column instead + scalar = ColGroup("scalar", None, "Scalar", subcols, download_col="ab") + self.assertEqual(scalar.download_subcols({}), []) + self.assertEqual(scalar.download({"a": 1, "b": 2, "ab": "12"}), "12") + + # Groups whose subcolumns depend on info are not expanded either; the ones we have + # (conjugacy class powers) set a download_col + callable_scalar = ColGroup("callable", None, "Callable", lambda info: subcols, + orig=["ab"], download_col="ab") + self.assertEqual(callable_scalar.download_subcols({}), []) + self.assertEqual(callable_scalar.download({"ab": "12"}), "12") + def test_integer_set(self): A = IntegerSet([2, 4]) B = IntegerSet([6, 9]) diff --git a/lmfdb/utils/downloader.py b/lmfdb/utils/downloader.py index 3ec88c87f7..e487de8497 100644 --- a/lmfdb/utils/downloader.py +++ b/lmfdb/utils/downloader.py @@ -1,7 +1,9 @@ """ -This file defines two kinds of classes used in constructing download files for the LMFDB: +This file defines three kinds of classes used in constructing download files for the LMFDB: * ``DownloadLanguage``, representing languages such as Sage and Magma +* ``ColumnSchema``, describing one downloaded column, shared between the places where a + download file describes its own contents * ``Downloader``, provides utility functions for downloading both search results and a single object. Can subclassed to provide customization. An instance of this class should be passed in as a download shortcut to the search_wrap constructor. @@ -182,9 +184,19 @@ def assign(self, name, inp): inp = self.to_lang(inp) return name + " " + self.assignment_defn + " " + inp + self.line_end + "\n" - def assign_columns(self, columns, column_names): - # We have a special function for assigning columns, to support adding hyperlinks to knowls in CSV files - return self.assign("columns", column_names) + def assign_columns(self, schema): + """ + Creates the assignment of the ``columns`` variable, listing the storage names + of the downloaded columns. + + We have a special function for this (rather than just calling ``assign``) since CSV + files have no comments and thus need to describe the columns in the header row itself. + + INPUT: + + - ``schema`` -- a list of ``ColumnSchema`` objects, one for each downloaded column + """ + return self.assign("columns", [entry.name for entry in schema]) def assign_iter(self, name, inp): """ @@ -382,14 +394,32 @@ def assign(self, name, inp): # Column assignments are handled separately below return "" - def assign_columns(self, columns, column_names): + def column_header(self, entry): + """ + The text of the header cell describing one downloaded column. + + Since CSV files have no comments, the grouped columns (which are downloaded as a + nested list of their subcolumns' values) are annotated with the ordered storage + names of that list's entries, so that the file describes its own contents (#6477). + We keep one header cell per top-level column, so the shape of the data is unchanged. + + INPUT: + + - ``entry`` -- a ``ColumnSchema`` object describing the column + """ + if entry.grouped: + return "%s [%s]" % (entry.name, ", ".join(sub.name for sub in entry.subcols)) + return entry.name + + def assign_columns(self, schema): urlparts = urlparse(request.url) out = [] - for col, name in zip(columns, column_names): + for entry in schema: + name = self.column_header(entry) # Make hyperlink of column name, if col.knowl exists - if getattr(col, "knowl", None): + if getattr(entry.col, "knowl", None): url = urlunparse(urlparts._replace( - path=url_for("knowledge.show", ID=col.knowl), + path=url_for("knowledge.show", ID=entry.col.knowl), params="", query="", fragment="" @@ -411,6 +441,38 @@ def to_lang_iter(self, inp): yield self.write(row) +class ColumnSchema(): + """ + A description of one column of a search results download. + + The columns of a download are described in several places (the comment at the top of the + file, the ``columns`` assignment, and the definitions at the bottom of the file), so we + build one of these for each column and share it between them. + + INPUT: + + - ``col`` -- the ``SearchCol`` being downloaded + - ``name`` -- a string, the storage name used for this column in the download file + - ``title`` -- a string, the title used when describing this column to a reader + - ``subcols`` -- for a grouped column that is downloaded as a nested list of its + subcolumns' values (see ``ColGroup.download``), the ``ColumnSchema`` objects describing + the entries of that list, in order. Empty for a column downloaded as a single value. + """ + def __init__(self, col, name, title, subcols=()): + self.col = col + self.name = name + self.title = title + self.subcols = list(subcols) + + @property + def grouped(self): + """ + Whether this column is downloaded as a nested list of its subcolumns' values, + rather than as a single value. + """ + return bool(self.subcols) + + class Downloader(): """ A class for downloading data in a uniform way. @@ -627,6 +689,41 @@ def get_sort(self, info, query): return S, display return None, None + def make_schema(self, cols, column_names, info): + """ + Describe the columns being downloaded, for use in the header comment, the ``columns`` + assignment and the definitions at the bottom of the download file. + + INPUT: + + - ``cols`` -- the list of search columns being downloaded + - ``column_names`` -- the corresponding storage names + - ``info`` -- the dictionary created from the url + + OUTPUT: + + A list of ``ColumnSchema`` objects, one for each downloaded column. + """ + def storage_name(col): + return col.name if col.download_col is None else col.download_col + + def disp_title(col, prefer_short=False): + # The (short) title used to describe a column in the download header and definitions + title = col.short_title if prefer_short else col.title + if title is None: + title = col.title + return title if isinstance(title, str) else title(info) + + schema = [] + for col, name in zip(cols, column_names): + # A ColGroup without a download_col is downloaded as a nested list of its subcolumns' + # values (see ColGroup.download), so we record those subcolumns in order to describe + # each of them individually (#6477). Every other column downloads as a single value. + subcols = [ColumnSchema(sub, storage_name(sub), disp_title(sub, prefer_short=True)) + for sub in col.download_subcols(info)] + schema.append(ColumnSchema(col, name, disp_title(col), subcols)) + return schema + def createrecord_code(self, lang, column_names): """ The contents of a function that creates a record from an entry of the data list. @@ -770,7 +867,11 @@ def __call__(self, info): seen.add(name) cols = [cols[i] for i in include] column_names = [column_names[i] for i in include] - data_format = [(col.title if isinstance(col.title, str) else col.title(info)) for col in cols] + + # One description of the downloaded columns, shared by the header comment, + # the columns assignment and the definitions at the bottom of the file + schema = self.make_schema(cols, column_names, info) + data_format = [entry.title for entry in schema] first50 = [[col.download(rec) for col in cols] for rec in first50] if num_results > 10000: # Estimate the size of the download file. This won't necessarily be a great estimate @@ -799,11 +900,19 @@ def make_download(): # We then describe the columns included, both in a comment and as a variable yield lang.comment(' Each entry in the following data list has the form:\n') yield lang.comment(' [' + ', '.join(data_format) + ']\n') + # Grouped columns are downloaded as a nested list of their subcolumns, so we spell + # out the contents of each such list here (#6477). + if any(entry.grouped for entry in schema): + yield lang.comment(' where the grouped columns are themselves lists:\n') + for entry in schema: + if entry.grouped: + yield lang.comment(' %s = [%s]\n' % ( + entry.title, ', '.join(sub.title for sub in entry.subcols))) yield lang.comment(' For more details, see the definitions at the bottom of the file.\n') if make_data_comment: yield lang.comment(f'\n {make_data_comment}\n') yield lang.comment('\n\n') - yield lang.assign_columns(cols, column_names) + yield lang.assign_columns(schema) # This is where the actual contents are included, applying postprocess and col.download to each yield from lang.assign_iter("data", lang.to_lang_iter( @@ -824,8 +933,12 @@ def make_download(): yield "\n" + lang.func_start("make_data", "") + self.makedata_code(lang) + lang.function_end + "\n\n" # We need to be able to look up knowls within knowls, so to reduce the number of database calls we just get them all. - # We do some global preprocessing to get access to knowls that define the columns - if any(col.download_desc is None for col in cols): + # We do some global preprocessing to get access to knowls that define the columns. + # Grouped columns (ColGroups downloaded as a nested list) are expanded into their + # subcolumns so that each subcolumn is documented individually (#6477). + if any(entry.col.download_desc is None + for top in schema + for entry in [top, *top.subcols]): from lmfdb.knowledge.knowl import knowldb all_knowls = {rec["id"]: (rec["title"], rec["content"]) for rec in knowldb.get_all_knowls(fields=["id", "title", "content"])} knowl_re = re.compile(r"""\{\{\s*KNOWL\(\s*["'](?:[^"']+)["'],\s*(?:title\s*=\s*)?['"]([^"']+)['"]\s*\)\s*\}\}""") @@ -839,12 +952,14 @@ def defines_subber(match): word = match.group(1) return f"**{word}**" - # If we haven't specified a more specific download_desc, we use the column knowl to get a string to add to the bottom of the file for each column - for col, name in zip(cols, column_names): + # If we haven't specified a more specific download_desc, we use the column knowl to get a + # string to add to the bottom of the file for each column + def emit_col_doc(entry): + col, name, disp_title = entry.col, entry.name, entry.title if col.download_desc is None: knowldata = all_knowls.get(col.knowl) if knowldata is None: - continue + return # We want to remove KNOWL and DEFINES macros _, content = knowldata knowl = knowl_re.sub(knowl_subber, content) @@ -854,14 +969,10 @@ def defines_subber(match): else: knowl = col.download_desc if knowl: - if isinstance(col.title, str): - title = col.title - else: - title = col.title(info) - if name.lower() == title.lower(): - yield lang.comment(f" {title} --\n") + if name.lower() == disp_title.lower(): + yield lang.comment(f" {disp_title} --\n") else: - yield lang.comment(f"{title} ({name}) --\n") + yield lang.comment(f"{disp_title} ({name}) --\n") for line in knowl.split("\n"): if line.strip(): yield lang.comment(" " + line.rstrip() + "\n") @@ -869,4 +980,14 @@ def defines_subber(match): yield lang.comment("\n") yield lang.comment("\n\n") + for entry in schema: + if entry.grouped: + # A grouped column, downloaded as a list; introduce it and document each subcolumn. + yield lang.comment(f" {entry.title} is a grouped column, downloaded as a list of the following subcolumns:\n\n") + yield from emit_col_doc(entry) + for sub in entry.subcols: + yield from emit_col_doc(sub) + else: + yield from emit_col_doc(entry) + return self._wrap_generator(make_download(), filename, lang=lang) diff --git a/lmfdb/utils/search_columns.py b/lmfdb/utils/search_columns.py index eec9333136..f5d62bc2fb 100644 --- a/lmfdb/utils/search_columns.py +++ b/lmfdb/utils/search_columns.py @@ -228,6 +228,20 @@ def download(self, rec): name = self.download_col return self._get(rec, name=name, downloading=True) + def download_subcols(self, info): + """ + Most columns produce a single value when downloading, so this returns an empty list. + + ``ColGroup`` overrides this: when it downloads as a nested list of its subcolumns' + values (see ``ColGroup.download``), it returns that list of subcolumns so that the + downloader can document each one individually. + + INPUT: + + - ``info`` -- the dictionary created from the url + """ + return [] + class SpacerCol(SearchCol): """ @@ -501,6 +515,16 @@ def download(self, rec): return self._get(rec, name=self.download_col, downloading=True) return [sub.download(rec) for sub in self.subcols] + def download_subcols(self, info): + # When a download_col is set, this group downloads as a single value (see download), + # so we behave like an ordinary column. Otherwise download produces a nested list + # over self.subcols, so we return those subcolumns to be documented individually. + # We only expand a concrete list of subcolumns (matching what download iterates over); + # if subcols is a callable we fall back to documenting the group as a whole. + if self.download_col is not None or callable(self.subcols): + return [] + return self.subcols + class SearchColumns: """