-
Notifications
You must be signed in to change notification settings - Fork 855
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.
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
Tabular: Fix error when loading with a different OS #2865
Conversation
Job PR-2865-17946e3 is done. |
Traceback (most recent call last): |
Thanks @imrooki, I plan to get a hold of a Windows machine to ensure this fix works correctly and have this fixed for v0.8 release. |
We look forward to the v0.8 release. |
@Innixma I've tested this PR locally, and it does solve the cross-OS load issue. Yet, there are still issues when calling Python version: 3.10.10 Test passed:
Test failed:
ps: __randomstate_ctor() should be a function in the |
17946e3
to
f7b0146
Compare
@eses-wk Hi, I think the error you got was because of some numpy version discrepancy between two environment. Also, I've updated the PR a bit and had a test myself across OS. Do you mind verifying the changes too? The change should be backward-compatible |
Job PR-2865-f7b0146 is done. |
Job PR-2865-709bc73 is done. |
@@ -0,0 +1,41 @@ | |||
from unittest.mock import patch |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should add a unit test for the absolute path example both from linux and windows
https://learn.microsoft.com/en-us/dotnet/standard/io/file-path-formats
For example, on windows it could be something like:
"C:\Documents\foo"
Job PR-2865-55a256b is done. |
Job PR-2865-1408fa0 is done. |
Job PR-2865-4edb33e is done. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM! @yinweisu feel free to approve and merge. I can't approve since I am the PR author.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Self approving :)
You were right, the issue was gone Sorry for the super late reply, I tried loading an old Window-trained model (Autogluon version==0.7.0) in a Linux machine (Autogluon version==0.7.1b20230606, installed from the latest master branch) and an For backward-compatibility:
|
Issue #, if available:
#2208
Description of changes:
Fixes exception during load when TabularPredictor was trained on Windows, and then loaded on MacOS/Linux.
Should also fix exception when trained on MacOS/Linux and loaded on Windows.
Because this PR introduces a new check on load based on a new variable, prior trained predictors on earlier versions will not be able to load using this PR.
Note: I have not formally tested this yet as I don't have a Windows machine on hand. Would appreciate if those affected can comment on if this PR fixes their problem.
Follow-up: Add unit test, tracked in #2863
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.