Skip to content

Add transform metadata to describe arrays' axes types #284

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom
Merged
inducer merged 3 commits into from
Jun 11, 2022
Merged

Add transform metadata to describe arrays' axes types #284

inducer merged 3 commits into from
Jun 11, 2022

Conversation

@kaushikcfd
Copy link
Collaborator

@kaushikcfd kaushikcfd commented Nov 8, 2021 •
edited by inducer
Loading

Draft because:

  • Fails CI.
  • ⚠️ DROP arraycontext req.txt commit before merging

@kaushikcfd kaushikcfd changed the title Add transform metadata to describe arrays' types Add transform metadata to describe arrays' axes types Nov 8, 2021
@kaushikcfd kaushikcfd requested a review from inducer November 8, 2021 21:22
inducer
inducer reviewed Nov 26, 2021
View reviewed changes
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! A few questions below.

@inducer
Copy link
Owner

inducer commented Nov 26, 2021

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

@kaushikcfd kaushikcfd force-pushed the axis_metadata branch 2 times, most recently from 0b9c42c to 26c9f8d Compare March 3, 2022 22:53
@kaushikcfd kaushikcfd requested a review from inducer March 3, 2022 23:07
inducer
inducer reviewed Mar 7, 2022
View reviewed changes
This was referenced Mar 12, 2022
Closed
Closed
inducer
inducer reviewed Mar 28, 2022
View reviewed changes
@kaushikcfd kaushikcfd force-pushed the axis_metadata branch 2 times, most recently from dde4973 to 58c429e Compare April 4, 2022 22:03
@kaushikcfd
Copy link
Collaborator Author

kaushikcfd commented Apr 4, 2022

@inducer: Thanks for the suggestions! I've made this patch leaner to make it more accurate.

@kaushikcfd kaushikcfd marked this pull request as draft April 4, 2022 22:36
@kaushikcfd kaushikcfd marked this pull request as ready for review April 5, 2022 06:11
@kaushikcfd kaushikcfd requested a review from inducer April 5, 2022 06:11
inducer
inducer reviewed Apr 17, 2022
View reviewed changes
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks! A few comments and suggestions below.

@inducer
Copy link
Owner

inducer commented Apr 17, 2022

Unsubscribing... @-mention or request review once it's ready for a look or needs attention.

inducer
inducer reviewed Jun 7, 2022
View reviewed changes
inducer
inducer reviewed Jun 7, 2022
View reviewed changes
@inducer inducer mentioned this pull request Jun 10, 2022
Merged
@inducer
Copy link
Owner

inducer commented Jun 10, 2022

FYI I'm working on getting this merge-ready, so no need to mess with it ATM. :)

inducer
inducer approved these changes Jun 10, 2022
View reviewed changes
Copy link
Owner

@inducer inducer left a comment

Choose a reason for hiding this comment

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

Thanks for working on this! I've made some changes to get this ship shape. @kaushikcfd Please take a look to see if you like my changes, and if so, this is ready to go. I can take care of updating inducer/grudge#188 afterwards.

@kaushikcfd
Copy link
Collaborator Author

kaushikcfd commented Jun 11, 2022 •
edited
Loading

Force-pushed to resolve conflicts with #334.

kaushikcfd
kaushikcfd commented Jun 11, 2022
View reviewed changes
Copy link
Collaborator Author

@kaushikcfd kaushikcfd left a comment

Choose a reason for hiding this comment

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

LGTM, thanks!

@inducer
Copy link
Owner

inducer commented Jun 11, 2022

Thanks for taking a look! Could you take a (hopefully quick) look at inducer/arraycontext#171, since this needs that?

@inducer
Copy link
Owner

inducer commented Jun 11, 2022

Argh, nvm! Was missing a reload. Sorry!

@inducer inducer enabled auto-merge (rebase) June 11, 2022 22:16
@inducer inducer merged commit dff7d4d into inducer:main Jun 11, 2022
@inducer inducer deleted the axis_metadata branch June 12, 2022 00:35
@inducer inducer mentioned this pull request Jun 12, 2022
Merged
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