Skip to content

Implement backwards-compatible shape and dims with Ellipsis support #4696

New issue

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

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

Already on GitHub? # to your account

Merged
merged 9 commits into from
Jun 4, 2021

Conversation

twiecki
Copy link
Member

@twiecki twiecki commented May 14, 2021

Here is what I did:
Create a diff between #4693 and #4667 and stage that diff onto a branch off of #4693 (no conflicts!). I then ran git commit -p and only selected those that seemed necessary here. Mostly there were some changes to tests that changed size to shape I did not commit.

Remember: Before merging still need to change author to @michaelosthege.

@twiecki twiecki changed the base branch from master to revert-shape-args May 14, 2021 07:08
@twiecki twiecki changed the title Reintro shape Reintroduce shape onto reversion PR May 14, 2021
@michaelosthege michaelosthege changed the title Reintroduce shape onto reversion PR Implement backwards-comparible shape and Ellipsis-enabled dims May 14, 2021
@michaelosthege michaelosthege changed the title Implement backwards-comparible shape and Ellipsis-enabled dims Implement backwards-compatible shape and Ellipsis-enabled dims May 14, 2021
@michaelosthege michaelosthege force-pushed the reintro_shape branch 2 times, most recently from 2f124df to 51a0f0b Compare May 14, 2021 22:33
@michaelosthege michaelosthege marked this pull request as ready for review May 14, 2021 23:32
@twiecki
Copy link
Member Author

twiecki commented May 15, 2021

I think this looks great!

@twiecki twiecki changed the base branch from revert-shape-args to v4 May 15, 2021 14:08
@twiecki
Copy link
Member Author

twiecki commented May 15, 2021

Just changed base to v4, not sure why we're getting conflicts there as #4693 was merged into v4?

@michaelosthege
Copy link
Member

After rebasing I'm now pushing this branch commit by commit, so we get CI results on all of them.

@twiecki
Copy link
Member Author

twiecki commented May 23, 2021 via email

michaelosthege and others added 7 commits June 4, 2021 17:10
Co-authored-by: Thomas Wiecki <thomas.wiecki@gmail.com>
Co-authored-by: Ricardo <ricardo.vieira1994@gmail.com>
Co-authored-by: Thomas Wiecki <thomas.wiecki@gmail.com>
Co-authored-by: Thomas Wiecki <thomas.wiecki@gmail.com>
Co-authored-by: Ricardo <ricardo.vieira1994@gmail.com>
Co-authored-by: Michael Osthege <m.osthege@fz-juelich.de>
@michaelosthege
Copy link
Member

As we decided in todays lab meeting is rebased this branch so we can merge it.

There were quite a few git conflicts, as anticipated, but it looks like nothing broke.

@twiecki
Copy link
Member Author

twiecki commented Jun 4, 2021 via email

OriolAbril
OriolAbril previously approved these changes Jun 4, 2021
Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
@twiecki twiecki merged commit f97b283 into v4 Jun 4, 2021
@twiecki twiecki deleted the reintro_shape branch June 4, 2021 21:00
@twiecki
Copy link
Member Author

twiecki commented Jun 5, 2021

Thanks to everyone who helped get this over the finish line, especially the herculean efforts of @michaelosthege! Also thanks for helpful review from @brandonwillard, @ricardoV94, @OriolAbril and everyone else.

# for free to join this conversation on GitHub. Already have an account? # to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants