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

Infer signature fix - no longer needed #23

Merged
merged 1 commit into from
Jan 20, 2025

Conversation

rayryeng
Copy link
Contributor

@rayryeng rayryeng commented Jan 17, 2025

Due to the recent updates to the scikit-learn and scipy packages in this pull request (#22), the infer_signature method found in src/train_random_forest/run.py is no longer required and does not need to be used in the mlflow.sklearn.save_model method. Leaving this alone will generate the following error (provided by a student):

59624797-009a-46ae-b37b-b409b4708dda-mobile

As such, this PR removes the infer_signature call to remove this error - it is no longer needed. This PR also updates one conda.yml file so that the scikit-learn version is set to 1.5.2, like all other conda.yml files in this repository. Finally, in the src/train_random_forest/run.py file, a hint refers to using the OneHotEncoder class, but it is not imported as part of import statements. Questions were also asked in this regard, so to make things slightly easier, this import statement was completed to be preemptive.

@rayryeng
Copy link
Contributor Author

CC: @SudKul

@rayryeng rayryeng force-pushed the infer_signature_fix branch from 41249cf to 5659fdf Compare January 17, 2025 16:01
@rayryeng
Copy link
Contributor Author

@islamalsawy

Can I have you look at this and merge it? This is the last piece of the puzzle that will allow for more seamless completion. This small piece is another issue that was very popular in the knowledge forums that I've had to continue resolving. With this, the amount of technical inquiries for completing this project should decrease.

@rayryeng rayryeng force-pushed the infer_signature_fix branch from 5659fdf to 900ab82 Compare January 20, 2025 14:10
@islamalsawy
Copy link
Member

@rayryeng Thanks Ray for working on it. I've merged it.

@islamalsawy islamalsawy merged commit 051a04e into udacity:main Jan 20, 2025
@rayryeng
Copy link
Contributor Author

@islamalsawy thank you for your speedy review!

@rayryeng rayryeng deleted the infer_signature_fix branch January 20, 2025 22:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants