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.

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

[ci] prevent lgb.model and lgb.pkl files being left behind after testing #6518

Merged

Conversation

nicklamiller
Copy link
Contributor

contributes to: #6361

Specificially prevents lgb.pkl and lgb.model files from being left behind after running tests by using pytest's tmp_path fixture. One can use the reprex described in #6361 (comment) to confirm these files are no longer left behind after these changes.

@nicklamiller nicklamiller force-pushed the use-tmp-paths-for-lgb-model-files branch from 262bfa8 to bed0201 Compare July 4, 2024 03:46
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks so much for working on this! Please see my first round of suggestions.

X, y = make_synthetic_regression()
params = {"objective": "regression", "verbose": -1}
num_train_rounds = 2
lgb_train = lgb.Dataset(X, y, free_raw_data=False)
bst = lgb.train(params=params, train_set=lgb_train, num_boost_round=num_train_rounds)
preds_raw = bst.predict(X, raw_score=True)
model_path_txt = str(tmp_path / "lgb.model")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Since this was already using tmp_path, I think it's very unlikely that this one was leaving a file behind. Could you please revert the changes to this case and any others that were already using tmp_path?

@jameslamb jameslamb changed the title Use tmp_path to prevent lgb.model and lgb.pkl files being left behind after testing [ci] Use tmp_path to prevent lgb.model and lgb.pkl files being left behind after testing Jul 4, 2024
@jameslamb jameslamb changed the title [ci] Use tmp_path to prevent lgb.model and lgb.pkl files being left behind after testing [ci] prevent lgb.model and lgb.pkl files being left behind after testing Jul 4, 2024
@jameslamb jameslamb self-requested a review July 5, 2024 02:48
Copy link
Collaborator

@jameslamb jameslamb left a comment

Choose a reason for hiding this comment

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

Thanks very much! I pulled this branch and ran the tests, confirmed that there now lgb.model and lgb.pkl aren't left behind.

@jameslamb jameslamb merged commit 3e6156a into microsoft:master Jul 5, 2024
41 checks passed
@nicklamiller nicklamiller deleted the use-tmp-paths-for-lgb-model-files branch July 5, 2024 17:06
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants