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

Recursive copying of attribute dictionaries for TensorImage subclass #3822

Merged
merged 2 commits into from Oct 21, 2022
Merged

Recursive copying of attribute dictionaries for TensorImage subclass #3822

merged 2 commits into from Oct 21, 2022

Conversation

restlessronin
Copy link
Contributor

@restlessronin restlessronin commented Oct 20, 2022

I ran into a problem with batch_to_sample when I was trying to apply it to multi-spectral images. I simplified the problematic code to the following

import torch
import fastai
from fastai.vision.all import *

class _T(TensorImage):
    pass

t = _T([[1.,2.],[3.,4.],[5.,6.]], kw=1)

test_eq(t.__dict__,{'kw': 1})

l = torch.unbind(t)

Basically I was expecting this to be true

test_eq(l[0].__dict__,{'kw': 1})

Instead I was getting

test_eq(l[0].__dict__,{})

If what I was expecting is the desired behaviour, then this branch contains my proposed fix. I'm a relative newbie to both python and fastai, so apologies if I'm missing something obvious.

I was also unable to get nbdev_test running properly on the fastai repo on my mac (lots of seemingly spurious test failures). Presumably the CI will run as part of this PR.

I would like to add my appreciation and admiration for Jeremy and the other incredible people who have helped create this community and movement. Well done!!!

@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@restlessronin
Copy link
Contributor Author

Modified the code to cover the edge case

t1 = _T([[1.,2.]], kw=1)
test_eq(t1.__dict__,{'kw': 1})
l1 = torch.unbind(t1)
test_eq(l1[0].__dict__,{'kw': 1})

@jph00
Copy link
Member

jph00 commented Oct 21, 2022

Nice PR - thanks!

@jph00 jph00 merged commit 6929df9 into fastai:master Oct 21, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants