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

Fix MNIST datasets img path column #3376

Merged
merged 1 commit into from
May 2, 2023
Merged

Conversation

shawnccx
Copy link
Contributor

@shawnccx shawnccx commented May 2, 2023

Use downloaded path instead of relative train/test path.

Code Pull Requests

Current MNIST example fails due to reading wrong paths of images.

[Errno 2] No such file or directory: '/Users/chongxiaoc/git/ludwig/examples/mnist/training/0/16585.png'
/Users/chongxiaoc/git/ludwig/ludwig/utils/image_utils.py:138: UserWarning: Failed to read image from PNG file. Original exception: Bytes object is empty. This could be due to a failed load from storage.
  warnings.warn(f"Failed to read image from PNG file. Original exception: {e}")
/Users/chongxiaoc/git/ludwig/ludwig/utils/image_utils.py:149: UserWarning: Failed to read image from numpy file. Original exception: Cannot load file containing pickled data when allow_pickle=False
  warnings.warn(f"Failed to read image from numpy file. Original exception: {e}")
/Users/chongxiaoc/git/ludwig/ludwig/utils/image_utils.py:120: UserWarning: Unable to read image from bytes object.
  warnings.warn("Unable to read image from bytes object.")

This PR fixes the img_path column in the dataframe.

Use downloaded path instead of relative train/test path.
@arnavgarg1
Copy link
Contributor

arnavgarg1 commented May 2, 2023

@shawnccx Nice catch! Thanks for your contribution! Approved!

@arnavgarg1 arnavgarg1 self-requested a review May 2, 2023 06:11
@github-actions
Copy link

github-actions bot commented May 2, 2023

Unit Test Results

       6 files  ±       0         6 suites  ±0   2h 51m 14s ⏱️ - 51m 15s
1 600 tests +1 446  1 566 ✔️ +1 430  33 💤 +15  1 +1 
1 633 runs  +1 305  1 593 ✔️ +1 307  39 💤  -   3  1 +1 

For more details on these failures, see this check.

Results for commit d1216dc. ± Comparison against base commit 662413e.

This pull request skips 5 tests.
tests.integration_tests.test_automl ‑ test_train_with_config_remote[s3]
tests.integration_tests.test_hyperopt ‑ test_hyperopt_sync_remote[s3]
tests.integration_tests.test_remote ‑ test_remote_training_set[s3-ray]
tests.regression_tests.benchmark.test_model_performance ‑ test_performance[ames_housing.gbm.yaml]
tests.regression_tests.benchmark.test_model_performance ‑ test_performance[mercedes_benz_greener.gbm.yaml]

@tgaddair tgaddair merged commit ffd8aca into ludwig-ai:master May 2, 2023
8 of 11 checks passed
@shawnccx
Copy link
Contributor Author

shawnccx commented May 2, 2023

@arnavgarg1 is it clean to merger?
The failed one test_model_save_reload_tv_model[googlenet-base] (tests.integration_tests.test_model_save_and_load) failed seems caused by http error and is not related with changes here.

@shawnccx
Copy link
Contributor Author

shawnccx commented May 2, 2023

@arnavgarg1 is it clean to merger? The failed one test_model_save_reload_tv_model[googlenet-base] (tests.integration_tests.test_model_save_and_load) failed seems caused by http error and is not related with changes here.

nvm, just saw it merged.

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.

None yet

4 participants