Skip to content

No inherited attrs - #520

Merged
inducer merged 2 commits into
mainfrom
no-inherited-attrs
Jul 17, 2024
Merged

inducer merged 2 commits into
mainfrom
no-inherited-attrs

Conversation

@inducer

@inducer inducer commented Jul 16, 2024

Copy link
Copy Markdown
Owner

No description provided.

@kaushikcfd kaushikcfd left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

Comment thread pytato/array.py

@attrs.frozen(eq=False, repr=False, hash=True, cache_hash=True)
class NamedArray(Array):
class NamedArray(_SuppliedAxesAndTagsMixin, Array):

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should this be "_SuppliedAxesAndTagsMixin"?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What do you mean? Are you saying this should not inherit from Array?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I meant NamedArray should not be taggable, instead it should propagate the tags and axes from the array it points to.

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The difficulty with that is that tags on NamedArrays can then no longer be changed (because we would have to change the DictOfNamedArrays, but that gets pointed to by potentially a bunch more NamedArrays.

Are we expecting that we can usefully call _with_new_tags on all Arrays?

Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

DistributedSendRefHolder also currently can't have its tags changed, but that at least could be implemented.

@inducer
inducer force-pushed the no-inherited-attrs branch from 21463da to be5942c Compare July 16, 2024 21:49
Comment thread pytato/distributed/nodes.py Outdated
@inducer
inducer force-pushed the no-inherited-attrs branch from be5942c to 22d6fd3 Compare July 17, 2024 21:16
@inducer
inducer enabled auto-merge (rebase) July 17, 2024 21:17
@inducer
inducer merged commit 5e22a98 into main Jul 17, 2024
@inducer
inducer deleted the no-inherited-attrs branch July 17, 2024 21:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants