#21765 closed New feature (fixed)

Contexts cannot be compared for equality.

Reported by: Keryn Knight <django@…> Owned by: onjin
Component: Template system Version: master
Severity: Normal Keywords:
Cc: Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Currently, comparing two contexts for equality requires knowing the internals of how Context is implemented -- ie: that there is a .dicts attribute available on an instance. For the purposes of writing unit tests, I needed to compare a context and a mutated one, and was surprised to find there's no concept of context equality:

>>> from django.template import Context
>>> Context() == Context()
False
>>> Context().dicts == Context().dicts
True
>>> Context({'x': 'y'}).dicts == Context({'x': 'y'}).dicts
True
>>> Context({'x': 'y'}).dicts == Context({'x': 'z'}).dicts
False
>>>

It possibly doesn't make sense for Context to have the gt/ge/lt/le magic methods defined on it (though one could argue they check the length of the .dicts), but from an API standpoint it might be nice to see eq/ne implemented for clean comparisons.

Tagging with master as the context doesn't seem to have changed since 1.6.1, which is what I'm testing against.

Change History (7)

comment:1 Changed 18 months ago by mjtamlyn

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset
  • Triage Stage changed from Unreviewed to Accepted

Seems a reasonable request to me.

comment:2 Changed 18 months ago by Keryn Knight <django@…>

As an implementation detail, should any context subclass be considered equal if it's underlying dataset is the same, such that Context({'x': 1}) == RenderContext({'x': 1}) and vice versa, or should type(x) == type(y) in addition to x.dicts == y.dicts? It's unclear what the best course of action would be, given there's no expectation of equality except my own.

comment:3 Changed 17 months ago by onjin

  • Owner changed from nobody to onjin
  • Status changed from new to assigned

comment:4 Changed 17 months ago by Baptiste Mispelon <bmispelon@…>

  • Resolution set to fixed
  • Status changed from assigned to closed

In d97bf2e9c8971764690caaf81a0914bc368d6b02:

Fixed #21765 -- Added support for comparing Context instances

comment:5 Changed 17 months ago by Keryn Knight <django@…>

  • Resolution fixed deleted
  • Status changed from closed to new

Re-opening with permission:
The fix as committed is not entirely correct, I don't think, as it only compares the last state change of a given context, so comparisons can incorrectly return as equal if context states drift and then re-align, for example:

>>> import django
>>> from django.template import Context
>>> a = Context()
>>> b = Context()
>>> a == b
True

the above is correct; from here on out, the contexts are continuations of the above example:

>>> a.update({'a': 1})
>>> a == b
False

that's also correct.

>>> a.update({'c': 3})
>>> b.update({'c': 3})
>>> a == b
True

This is where the incorrectness appears. They are not equal, but their last push to the stack was, so they're being treated as equal. Based on the commit in question (d97bf2e9c8971764690caaf81a0914bc368d6b02), the change would be going from:

return self.dicts[-1] == other.dicts[-1]

to

return self.dicts == other.dicts

which would at least ensure the rest of the context is taken into account, though that fix in itself may not be as correct as it should be -- it is strict in the sense that the entire history of the contexts being compared must be aligned, where it should perhaps instead squash the stack into one dictionary for both sides and compare them for equality.

(Whether or not strictness is a benefit or problem is probably a DDN, and like testing equality for context subclasses which differ (or have different attrs, for things like RenderContext) could probably be tracked in separate tickets, though.)

comment:6 Changed 17 months ago by Baptiste Mispelon <bmispelon@…>

In 8274fa60f88607eb0e81908f049c391455956dd8:

Made the new template.Context.flatten() method a public API.

That method was introduced in 9db4271bd11ac23a5a5652bbcdf8fb6d4b997651.

Refs #21765.

comment:7 Changed 15 months ago by leotin

  • Resolution set to fixed
  • Status changed from new to closed

It seems like the reported issue is finally fixed after the update by 8274fa60f88607eb0e81908f049c391455956dd8.

Note: See TracTickets for help on using tickets.
Back to Top