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

Remove Print in Groupby #180

Merged
merged 1 commit into from
Jun 24, 2024
Merged

Remove Print in Groupby #180

merged 1 commit into from
Jun 24, 2024

Conversation

michaelmckinsey1
Copy link
Collaborator

@michaelmckinsey1 michaelmckinsey1 commented Jun 20, 2024 •
edited
Loading

There's a print in Thicket.groupby() that really shouldn't be there. It prints the amount of Thickets created and the dictionary of key to Thicket object.

Reasons why it should be removed:

  1. pandas groupby doesn't do this.
  2. The resulting GroupBy object can be printed after it's created anyway, as our GroupBy class inherits from dict

@michaelmckinsey1 michaelmckinsey1 added area-thicket Issues and PRs involving Thicket's core Thicket datastructure and associated classes priority-normal Normal priority issues and PRs status-ready-for-review This PR is ready to be reviewed by assigned reviewers type-internal-cleanup PRs or Issues related to the structure of the codebase, directories, and refactors labels Jun 20, 2024
@michaelmckinsey1 michaelmckinsey1 self-assigned this Jun 20, 2024
pearce8
pearce8 requested changes Jun 20, 2024
View reviewed changes
Copy link
Collaborator

@pearce8 pearce8 left a comment

Choose a reason for hiding this comment

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

Fix unit tests

@michaelmckinsey1 michaelmckinsey1 requested a review from pearce8 June 24, 2024 19:24
@pearce8 pearce8 merged commit fc929c5 into LLNL:develop Jun 24, 2024
4 checks passed
michaelmckinsey1 added a commit to michaelmckinsey1/thicket that referenced this pull request Jul 9, 2024
@michaelmckinsey1
Remove print (LLNL#180)
815c0df
michaelmckinsey1 added a commit to michaelmckinsey1/thicket that referenced this pull request Jul 9, 2024
@michaelmckinsey1
Remove print (LLNL#180)
b6af072
Yejashi pushed a commit to TauferLab/thicket that referenced this pull request Sep 4, 2024
@michaelmckinsey1 @Yejashi
Remove print (LLNL#180)
55ddeb4
@slabasan slabasan added this to the 2024.2.0 milestone Sep 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pearce8 pearce8 pearce8 approved these changes

@ilumsden ilumsden Awaiting requested review from ilumsden

Assignees

@michaelmckinsey1 michaelmckinsey1

Labels
area-thicket Issues and PRs involving Thicket's core Thicket datastructure and associated classes priority-normal Normal priority issues and PRs status-ready-for-review This PR is ready to be reviewed by assigned reviewers type-internal-cleanup PRs or Issues related to the structure of the codebase, directories, and refactors
Projects
None yet
Milestone
2024.2.0
Development

Successfully merging this pull request may close these issues.
3 participants
@michaelmckinsey1 @pearce8 @slabasan