Allow datagrid Column subclasses to set attributes on the class level.

Review Request #14824 — Created Feb. 17, 2026 and updated

Information

Djblets
release-5.x

Reviewers

Historically, Column subclasses had to override __init__ and set
attributes there. This was a bit messy and error-prone, as it was common
to mix up positional and keyword arguments when subclassing, and made
for harder customization for more complex class hierarchies.

Now, subclasses can set these arguments directly on the class as
attributes. The base Column class has defaults for all these fields,
which can be overridden by subclasses and further overridden when
calling __init__(). Built-in subclasses also provide their own fitting
this pattern.

To make this work, the arguments in __init__() are all set to UNSET
by default. If UNSET, class attributes (or more complex default
calculations) are used, but if any value is explicitly provided, it will
be used instead.

To help avoid the mess of positional and keyword arguments in these
classes, all arguments other than label are now keyword-only, with
positional arguments deprecated and scheduled for removal in Djblets 7.

This is now the recommended approach for column subclasses. It has the
benefit of storing less state per-instance, as well, as we only set the
values on the instance if they're explicitly provided during
construction.

As an added bonus, we've deprecated cell_clickable which is confusing
and has never been largely ignored by the Djblets code. It's used by
the template to inject an onclick="javascript:window.location = ...",
but there's no way to set it without overriding it on a column instance.

Unit tests pass.

Tested updating existing columns to use this, and verified the new
settings were correct.

Summary ID
Allow datagrid Column subclasses to set attributes on the class level.
Historically, `Column` subclasses had to override `__init__` and set attributes there. This was a bit messy and error-prone, as it was common to mix up positional and keyword arguments when subclassing, and made for harder customization for more complex class hierarchies. Now, subclasses can set these arguments directly on the class as attributes. The base `Column` class has defaults for all these fields, which can be overridden by subclasses and further overridden when calling `__init__()`. Built-in subclasses also provide their own fitting this pattern. To make this work, the arguments in `__init__()` are all set to `UNSET` by default. If `UNSET`, class attributes (or more complex default calculations) are used, but if any value is explicitly provided, it will be used instead. To help avoid the mess of positional and keyword arguments in these classes, all arguments other than `label` are now keyword-only, with positional arguments deprecated and scheduled for removal in Djblets 7. This is now the recommended approach for column subclasses. It has the benefit of storing less state per-instance, as well, as we only set the values on the instance if they're explicitly provided during construction. As an added bonus, we've deprecated `cell_clickable` which is confusing and has never been largely ignored by the Djblets code. It's used by the template to inject an `onclick="javascript:window.location = ..."`, but there's no way to set it without overriding it on a column instance.
c7feb1be9cedf8c56814fb0e9e0cd613fc328214
Description From Last Updated

Should we do format_html(self.detailed_label) here to be safe?

maubinmaubin

Can you add a Raises section for this in the docstring.

maubinmaubin

How come we're setting this to False instead of using the cell_clickable arg?

maubinmaubin

This is a no-op. We should just have: if detailed_label is UNSET: detailed_label = self.detailed_label

daviddavid

In DateTimeSinceColumn you included label in the kwarg-only section. Should we do that here too?

daviddavid

This needs to come after the *args docstring.

maubinmaubin

I think we still want to pass label positionally to the parent __init__(), and then pass sortable=sortable after *args.

maubinmaubin

Can we emit a DeprecationWarning if this is passed in?

daviddavid

This class doesn't have a test case to test the init method, like the others do.

daviddavid

Copy-pasteo: DateTimeColumn -> DateTimeSinceColumn

daviddavid

Can we assert on label in here as well?

daviddavid

Can we add 'label': 'My Column' to this? (this would catch the label issue that Michelle flagged)

daviddavid

We might want to make this if not (self.label or label):, because otherwise if label is passed in, we do …

daviddavid
maubin
  1. 
      
  2. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues

    Should we do format_html(self.detailed_label) here to be safe?

  3. djblets/datagrid/grids.py (Diff revision 1)
     
     
     
     
     
    Show all issues

    Can you add a Raises section for this in the docstring.

  4. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues

    How come we're setting this to False instead of using the cell_clickable arg?

    1. Had to look into this, because it's what we were doing before.

      This attribute controls whether we do a onclick="javascript:window.location = ...", which I remember from ancient times, and oh boy is it ancient. 2007. And it's been hard-coded to False since it was introduced.

      This basically has just never been used, and we should just deprecate the field entirely. But I want to hold off on that in this change.

  5. 
      
david
  1. 
      
  2. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues

    This is a no-op. We should just have:

    if detailed_label is UNSET:
        detailed_label = self.detailed_label
    
    1. Ah thanks. I had iterated on that code a bit.

  3. djblets/datagrid/grids.py (Diff revision 1)
     
     
    Show all issues

    In DateTimeSinceColumn you included label in the kwarg-only section. Should we do that here too?

    1. That was a mistake. label is positional. It should just be in *args, but that'll have to wait for the deprecation to be complete.

  4. 
      
chipx86
maubin
  1. 
      
  2. djblets/datagrid/grids.py (Diff revisions 1 - 2)
     
     
     
     
     
    Show all issues

    This needs to come after the *args docstring.

  3. djblets/datagrid/grids.py (Diff revisions 1 - 2)
     
     
     
    Show all issues

    I think we still want to pass label positionally to the parent __init__(), and then pass sortable=sortable after *args.

  4. 
      
david
  1. 
      
  2. djblets/datagrid/grids.py (Diff revision 2)
     
     
    Show all issues

    Can we emit a DeprecationWarning if this is passed in?

  3. djblets/datagrid/tests.py (Diff revision 2)
     
     
    Show all issues

    This class doesn't have a test case to test the init method, like the others do.

  4. djblets/datagrid/tests.py (Diff revision 2)
     
     
     
     
     
     
    Show all issues

    Can we add 'label': 'My Column' to this? (this would catch the label issue that Michelle flagged)

  5. 
      
chipx86
maubin
  1. Ship It!
  2. 
      
david
  1. 
      
  2. djblets/datagrid/tests.py (Diff revisions 2 - 3)
     
     
    Show all issues

    Copy-pasteo: DateTimeColumn -> DateTimeSinceColumn

  3. djblets/datagrid/tests.py (Diff revisions 2 - 3)
     
     
    Show all issues

    Can we assert on label in here as well?

  4. djblets/datagrid/grids.py (Diff revision 3)
     
     
    Show all issues

    We might want to make this if not (self.label or label):, because otherwise if label is passed in, we do this format_html and then immediately overwrite it in super().__init__()

  5. 
      
chipx86
Review request changed
Change Summary:
  • Fixed an error where a custom label for CheckboxColumn would get overridden, and added tests.
  • Added an assertion for a DateTimeSinceColumn's label.
  • Fixed docstring issues.
Commits:
Summary ID
Allow datagrid Column subclasses to set attributes on the class level.
Historically, `Column` subclasses had to override `__init__` and set attributes there. This was a bit messy and error-prone, as it was common to mix up positional and keyword arguments when subclassing, and made for harder customization for more complex class hierarchies. Now, subclasses can set these arguments directly on the class as attributes. The base `Column` class has defaults for all these fields, which can be overridden by subclasses and further overridden when calling `__init__()`. Built-in subclasses also provide their own fitting this pattern. To make this work, the arguments in `__init__()` are all set to `UNSET` by default. If `UNSET`, class attributes (or more complex default calculations) are used, but if any value is explicitly provided, it will be used instead. To help avoid the mess of positional and keyword arguments in these classes, all arguments other than `label` are now keyword-only, with positional arguments deprecated and scheduled for removal in Djblets 7. This is now the recommended approach for column subclasses. It has the benefit of storing less state per-instance, as well, as we only set the values on the instance if they're explicitly provided during construction. As an added bonus, we've deprecated `cell_clickable` which is confusing and has never been largely ignored by the Djblets code. It's used by the template to inject an `onclick="javascript:window.location = ..."`, but there's no way to set it without overriding it on a column instance.
e2abeb2b206fbe0186d2dde3a65824b82c8b6076
Allow datagrid Column subclasses to set attributes on the class level.
Historically, `Column` subclasses had to override `__init__` and set attributes there. This was a bit messy and error-prone, as it was common to mix up positional and keyword arguments when subclassing, and made for harder customization for more complex class hierarchies. Now, subclasses can set these arguments directly on the class as attributes. The base `Column` class has defaults for all these fields, which can be overridden by subclasses and further overridden when calling `__init__()`. Built-in subclasses also provide their own fitting this pattern. To make this work, the arguments in `__init__()` are all set to `UNSET` by default. If `UNSET`, class attributes (or more complex default calculations) are used, but if any value is explicitly provided, it will be used instead. To help avoid the mess of positional and keyword arguments in these classes, all arguments other than `label` are now keyword-only, with positional arguments deprecated and scheduled for removal in Djblets 7. This is now the recommended approach for column subclasses. It has the benefit of storing less state per-instance, as well, as we only set the values on the instance if they're explicitly provided during construction. As an added bonus, we've deprecated `cell_clickable` which is confusing and has never been largely ignored by the Djblets code. It's used by the template to inject an `onclick="javascript:window.location = ..."`, but there's no way to set it without overriding it on a column instance.
c7feb1be9cedf8c56814fb0e9e0cd613fc328214

Checks run (2 succeeded)

flake8 passed.
JSHint passed.
maubin
  1. Ship It!
  2.