-
Notifications
You must be signed in to change notification settings - Fork 6.8k
Fix transform call in ImageFolderDataset #15672
base: master
Are you sure you want to change the base?
Conversation
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.
@john- Thank you for the fix, I'll merge it next week.
@john- the tests seem to be stuck, can you please merge master to you branch and push to trigger new set of tests ? Thanks! |
I am not quite sure what I am doing but so far I have:
When you say push to trigger a new set of tests does that mean do another pull request? |
@john- thank you, I'll try to get the tests running |
@szha Hi there, could you check when you have time why tests are stuck on this pr ? |
@mxnet-label-bot Add [Perl, Gluon] |
Thanks for the contribution @john- |
@john- gentle ping to trigger CI. thanks! |
Do I need to:
I don't see how doing the 'git commit --allow-empty -m "Trigger notification"' will do anything otherwise. Also, note that I merged master back into my branch. |
I did the trigger notification commit and pushed it to my branch. I see it listed as a commit in the pull request. Should I do something else to make this work? |
I read through the Details for the ci failures and it is not clear to me how the errors relate to the pull request. Can someone provide some insight? Maybe I need to re-merge the master branch? |
Description
Without this change I get the following (partial) error:
Expected exactly one or two argument for an accessor of transform at
/home/user/perl5/lib/perl5/AI/MXNet/Gluon/Data/Vision.pm line 422, line 207.
This change addresses that error.
Checklist
Essentials
I will take a look at how to add unit test to this change.
Changes
There are no new features. From my perspective the change addresses a typo as other packages in the same file call the transform sub the same way this change does.
Comments
I could not get original code to work so cannot confirm if change is backwards compatible.