8000 [core] Move AbstractTabularTrainer to tabular by canerturkmen · Pull Request #4851 · autogluon/autogluon · GitHub
[go: up one dir, main page]
Skip to content

[core] Move AbstractTabularTrainer to tabular #4851

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
merged 10 commits into from
Feb 7, 2025
Merged

[core] Move AbstractTabularTrainer to tabular #4851

merged 10 commits into from
Feb 7, 2025

Conversation

canerturkmen
Copy link
Contributor
@canerturkmen canerturkmen commented Jan 30, 2025 •
edited
Loading

Issue #, if available:

Description of changes:

  • Minor typing tweaks in AbstractTrainer, TimeSeriesTrainer
  • Minor edits in AbstractTabularTrainer: formatting, typing additions to method signatures or removing dead code.

Note: After your approval I will move the AbstractTabularTrainer out of core and into tabular. I am opening the PR in this state to make code review easier. @shchur @Innixma

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

@canerturkmen canerturkmen added code cleanup Fixing warnings/deprecations/syntax/etc. module: core module: tabular module: timeseries related to the timeseries module labels Jan 30, 2025
shchur
shchur reviewed Jan 30, 2025
View reviewed changes
@github-actions GitHub Actions
Copy link

Job PR-4851-bd24491 is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-4851/bd24491/index.html

Innixma
Innixma reviewed Jan 30, 2025
View reviewed changes
Innixma
Innixma reviewed Jan 31, 2025
View reviewed changes
Innixma
Innixma requested changes Jan 31, 2025
View reviewed changes
Copy link
Contributor
@Innixma Innixma left a comment

Choose a reason for hiding this comment

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

Looks good to me, but I'd like clarification on why we are deleting the parallal model refit code

@canerturkmen canerturkmen force-pushed the move-abstract-trainer branch from bd24491 to 0a8ea44 Compare February 3, 2025 13:25
@canerturkmen canerturkmen marked this pull request as ready for review February 3, 2025 14:40
@canerturkmen
Copy link
Contributor Author

@Innixma please also note the change in log_utils.py, i.e., tabular will have to re-wrap the utility to also silence the trainer logs.

@github-actions GitHub Actions
Copy link
github-actions bot commented Feb 3, 2025

Job PR-4851-cb446d1 is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-4851/cb446d1/index.html

shchur
shchur approved these changes Feb 4, 2025
View reviewed changes
Copy link
Collaborator
@shchur shchur left a comment

Choose a reason for hiding this comment

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

LGTM!

@canerturkmen canerturkmen force-pushed the move-abstract-trainer branch from cb446d1 to 9ff1c98 Compare February 6, 2025 13:09
@github-actions GitHub Actions
Copy link
github-actions bot commented Feb 6, 2025

Job PR-4851-9ff1c98 is done.
Docs are uploaded to http://autogluon-staging.s3-website-us-west-2.amazonaws.com/PR-4851/9ff1c98/index.html

Innixma
Innixma approved these changes Feb 7, 2025
View reviewed changes
Copy link
Contributor
@Innixma Innixma left a comment

Choose a reason for hiding this comment

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

LGTM, awesome work!

@Innixma Innixma merged commit 0d2e3a3 into autogluon:master Feb 7, 2025
27 checks passed
@shchur shchur added this to the 1.3 Release milestone Apr 11, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@shchur shchur shchur approved these changes

@Innixma Innixma Innixma approved these changes

Assignees
No one assigned
Labels
code cleanup Fixing warnings/deprecations/syntax/etc. module: core module: tabular module: timeseries related to the timeseries module
Projects
None yet
Milestone
1.3 Release
Development

Successfully merging this pull request may close these issues.
3 participants
@canerturkmen @shchur @Innixma
0