number_distributed_tags: sort tags by repr

matthiasdiener commented 7 months ago

cc @MTcam

Should resolve issues like

TypeError: '<' not supported between instances of 'type' and 'type'

Followup to #462.

kaushikcfd commented 7 months ago

The repr should be consistent too. I don't think Record.__repr__ is same across interpreter runs.

On Thu, Nov 9, 2023, 12:16 PM Andreas Klöckner @.***> wrote:

@.**** commented on this pull request.

In pytato/distributed/tags.py https://github.com/inducer/pytato/pull/467#discussion_r1388538180:

@@ -106,7 +106,7 @@ def set_union( next_tag = base_tag assert isinstance(all_tags, frozenset)

for sym_tag in sorted(all_tags):

for sym_tag in sorted(all_tags, key=lambda tag: repr(tag)):

This now requires that tags have a textual representation... so please update the docs.

— Reply to this email directly, view it on GitHub https://github.com/inducer/pytato/pull/467#pullrequestreview-1723423798, or unsubscribe https://github.com/notifications/unsubscribe-auth/ADVPQYQTRIEQUZOXEBLVNTDYDU22ZAVCNFSM6AAAAAA7FAWH52VHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTOMRTGQZDGNZZHA . You are receiving this because you are subscribed to this thread.Message ID: @.***>

matthiasdiener commented 7 months ago

The repr should be consistent too. I don't think Record.__repr__ is same across interpreter runs.

Yes, that's true, and also applies to frozenset etc. Between this approach and #469, I'm not sure which one is better.

matthiasdiener commented 4 months ago

Closing in favor of #469.

inducer / pytato

number_distributed_tags: sort tags by repr #467

@.**** commented on this pull request.