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

Fixed cfproto_cat_adult_ohe example #750

Merged

Conversation

RobertSamoilescu
Copy link
Collaborator

@RobertSamoilescu RobertSamoilescu commented Sep 9, 2022

Replaced OHE sparse with OHE dense and fixed display function.
Addresses #749

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov
Copy link

codecov bot commented Sep 9, 2022

Codecov Report

Merging #750 (b252075) into master (e95e66b) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #750   +/-   ##
=======================================
  Coverage   80.96%   80.96%           
=======================================
  Files         105      105           
  Lines       11876    11876           
=======================================
  Hits         9615     9615           
  Misses       2261     2261           

@jklaise
Copy link
Member

jklaise commented Sep 9, 2022

Testing this locally, this is not triggered with numpy version 1.20.3 but it is with version 1.22.4. Since the error originates from a call to np.percentile it appears that earlier numpy versions may have been able to handle np.matrix (which is the result of conversion from sparse to dense matrix in the original notebook) but no longer so.

Copy link
Member

@jklaise jklaise left a comment

Choose a reason for hiding this comment

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

Thanks Robert, have tested this locally as well.

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

2 participants