-
Notifications
You must be signed in to change notification settings - Fork 7.5k
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
Check if normalize exists on _add_norm
#3371
Conversation
Check out this pull request on See visual diffs & provide feedback on Jupyter Notebooks. Powered by ReviewNB |
Very nice fix and test! cc: @jph00 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great - just a couple of minor suggestions. Please at-mention me when ready to go
Co-authored-by: Jeremy Howard <github@jhoward.fastmail.fm>
Co-authored-by: Jeremy Howard <github@jhoward.fastmail.fm>
Ready to go @jph00, updated with your suggestions. |
I’ve been made aware of that this only adds Normalize to the train, not the
validation. Can you confirm this Renato? And perhaps maybe we can fix that
here?
…On Sun, May 16, 2021 at 6:38 AM Renato Hermoza Aragonés < ***@***.***> wrote:
Ready to go @jph00 <https://github.com/jph00>, updated with your
suggestions.
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#3371 (comment)>, or
unsubscribe
<https://github.com/notifications/unsubscribe-auth/AB3YCV6EKPBAMEY54CAM763TN6OARANCNFSM44YI2HVA>
.
|
@muellerzr I just tried and it was working fine. Can you share and example of that happening? |
@renato145 thanks for the verification, I believe this may have also been fixed in this latest version (the issue opened up on the forums). Also looks like notebooks and the lib aren't synced. Can you run |
Many thanks :) |
Fixes #3370