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

MAINT: replace deprecated boston dataset with diabetes #870

Merged
merged 1 commit into from
Apr 3, 2023

Conversation

homosapien-lcy
Copy link
Contributor

@homosapien-lcy homosapien-lcy commented Mar 31, 2023

The load_boston dataset is deprecated, replacing with the diabetes dataset (seems to the the only usable dataset in sklearn.dataset, iris and california housing dataset is not compatible with the code).

However, there are two more errors arised after the fix that I found the fixes but want to discuss:

After correcting the dataset, an error will happen in get_image_from_file() since this function tries to read an image in the demo/basic/capitol.jpg, but the folder doesn't seem to exist anymore, removing this function can fix this problem.

Then, another error will happend in chaco.chaco.plots.polar_line_renderer, in function _draw_plot(). Function will try to call _render on gc (line 136), which is a parameter not exists. Removing this line can solve the problem (this also causes exception in issue #875 )

When both the above fixes are done, the plots can be generated without error. But are these fixes sounds good to you?

@dpinte
Copy link
Member

dpinte commented Mar 31, 2023

@dpinte
Copy link
Member

dpinte commented Mar 31, 2023

@homosapien-lcy you should update the code to use the importlib.resources to access the file.

@homosapien-lcy
Copy link
Contributor Author

Will use importlib_resource to find capitol.jpg

@dpinte
Copy link
Member

dpinte commented Mar 31, 2023

@homosapien-lcy by the way, I think it's important to split the issues into different PR's. Everything related to fixing the create_plot_snapshot can be done in this PR (but then the PR must be renamed). The issue with polar renderer is not a problem with the demo but a bug in chaco. That should be a different PR!

@homosapien-lcy
Copy link
Contributor Author

homosapien-lcy commented Apr 3, 2023

@homosapien-lcy by the way, I think it's important to split the issues into different PR's. Everything related to fixing the create_plot_snapshot can be done in this PR (but then the PR must be renamed). The issue with polar renderer is not a problem with the demo but a bug in chaco. That should be a different PR!

Got it, separating into two other issues #879 #881

Copy link
Member

@dpinte dpinte left a comment

Choose a reason for hiding this comment

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

LGTM

@dpinte dpinte merged commit e69ba4b into main Apr 3, 2023
@dpinte dpinte deleted the boston_dataset_fix branch April 3, 2023 07:58
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