Skip to content

Implement WalkMapperWithExtraArgs - #387

Merged
inducer merged 4 commits into
mainfrom
walk_mapper_extra_args
Nov 9, 2022
Merged

Implement WalkMapperWithExtraArgs#387
inducer merged 4 commits into
mainfrom
walk_mapper_extra_args

Conversation

@kaushikcfd

@kaushikcfd kaushikcfd commented Nov 7, 2022 •
edited by inducer
Loading

Copy link
Copy Markdown
Collaborator

Best read commit-by-commit.

Draft because:

@kaushikcfd
kaushikcfd requested a review from inducer November 7, 2022 22:53
inducer
inducer reviewed Nov 7, 2022
View reviewed changes
Comment thread pytato/transform/__init__.py Outdated
@inducer inducer mentioned this pull request Nov 8, 2022
Merged
@kaushikcfd kaushikcfd mentioned this pull request Nov 9, 2022
Closed
1 task
@kaushikcfd
kaushikcfd force-pushed the walk_mapper_extra_args branch from 239f95a to d264ae9 Compare November 9, 2022 06:31
@kaushikcfd
kaushikcfd requested a review from inducer November 9, 2022 06:34
inducer
inducer reviewed Nov 9, 2022
View reviewed changes

@inducer inducer left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks! Some minor questions, otherwise this is headed in the right direction.

Comment thread pytato/transform/__init__.py Outdated
Comment thread pytato/transform/__init__.py Outdated
@kaushikcfd
kaushikcfd force-pushed the walk_mapper_extra_args branch from d264ae9 to f529985 Compare November 9, 2022 14:41
@kaushikcfd kaushikcfd mentioned this pull request Nov 9, 2022
Merged
@kaushikcfd
kaushikcfd force-pushed the walk_mapper_extra_args branch from f529985 to aa80b3f Compare November 9, 2022 15:47
@kaushikcfd
kaushikcfd marked this pull request as draft November 9, 2022 15:47
@kaushikcfd
kaushikcfd force-pushed the walk_mapper_extra_args branch from c1b64d8 to 8064998 Compare November 9, 2022 16:38
@kaushikcfd

kaushikcfd commented Nov 9, 2022 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

I'll handle the conflicts here.

Edit: Rebased!

@kaushikcfd
kaushikcfd force-pushed the walk_mapper_extra_args branch from 8064998 to 0b0800c Compare November 9, 2022 18:03
@kaushikcfd
kaushikcfd marked this pull request as ready for review November 9, 2022 18:03
inducer
inducer reviewed Nov 9, 2022
View reviewed changes

@inducer inducer left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks! A few (hopefully small) wrinkles, then this is good to go.

Comment thread pytato/transform/__init__.py Outdated
Comment thread pytato/transform/__init__.py Outdated
Comment thread pytato/partition.py Outdated
Comment thread pytato/analysis/__init__.py
@kaushikcfd
kaushikcfd force-pushed the walk_mapper_extra_args branch from 0b0800c to b93c3fd Compare November 9, 2022 19:18
@kaushikcfd
kaushikcfd force-pushed the walk_mapper_extra_args branch from b93c3fd to 67ba18a Compare November 9, 2022 19:29
inducer
inducer approved these changes Nov 9, 2022
View reviewed changes

@inducer inducer left a comment

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Thanks!

@inducer
inducer enabled auto-merge (rebase) November 9, 2022 19:33
@inducer

inducer commented Nov 9, 2022

Copy link
Copy Markdown
Owner

I wish there was a cleaner way to tell the type checker about the dropped *args and **kwargs, but 🤷 I can't think of anything.

@inducer
inducer merged commit 2d94472 into main Nov 9, 2022
@inducer
inducer deleted the walk_mapper_extra_args branch November 9, 2022 20:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@inducer inducer inducer approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@kaushikcfd @inducer