Skip to content
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

Cadl 41 py311 integration #479

Merged
merged 5 commits into from
Feb 6, 2025
Merged

Cadl 41 py311 integration #479

merged 5 commits into from
Feb 6, 2025

Conversation

angelo-romano
Copy link
Contributor

No description provided.

@angelo-romano angelo-romano force-pushed the CADL-41-py311-integration branch 14 times, most recently from f5a9ad7 to 80ed2a3 Compare January 28, 2025 15:02
dhandy2013
dhandy2013 reviewed Jan 28, 2025
View reviewed changes
Copy link
Contributor

@dhandy2013 dhandy2013 left a comment

Choose a reason for hiding this comment

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

My general feedback so far: Can you break up this PR into two parts, the scrunch improvements and the py311 tests you are adding?

There was a huge number of lines changed due to black formatting of scrunch/datasets.py. Because there are so many changes it is hard to tell which are just formatting and which might be substantial.

@angelo-romano angelo-romano force-pushed the CADL-41-py311-integration branch from 80ed2a3 to df9aa40 Compare January 29, 2025 08:49
@angelo-romano
Copy link
Contributor Author

My general feedback so far: Can you break up this PR into two parts, the scrunch improvements and the py311 tests you are adding?

There was a huge number of lines changed due to black formatting of scrunch/datasets.py. Because there are so many changes it is hard to tell which are just formatting and which might be substantial.

Done. The code cleanup parts are now under #480 (CADL-47)

@angelo-romano angelo-romano force-pushed the CADL-41-py311-integration branch from b39dc8c to 14901ff Compare January 30, 2025 11:28
@angelo-romano angelo-romano force-pushed the CADL-41-py311-integration branch from 90e8e2b to fad943e Compare February 3, 2025 12:27
@dhandy2013 dhandy2013 merged commit 2a170eb into master Feb 6, 2025
5 checks passed
percious added a commit that referenced this pull request Feb 6, 2025
@percious
Revert "Merge pull request #479 from Crunch-io/CADL-41-py311-integrat…
755ed71 
…ion"

This reverts commit 2a170eb, reversing
changes made to 794e2ea.
percious added a commit that referenced this pull request Feb 6, 2025
@percious
Merge pull request #481 from Crunch-io/revert-PR-479
835a8ce 
Revert "Merge pull request #479 from Crunch-io/CADL-41-py311-integrat…
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@dhandy2013 dhandy2013 dhandy2013 approved these changes

@mpanczyk mpanczyk mpanczyk 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.
3 participants
@angelo-romano @dhandy2013 @mpanczyk