search

tmabraham / UPIT

A fastai/PyTorch package for unpaired image-to-image translation.
https://tmabraham.github.io/UPIT
Apache License 2.0
133 stars 21 forks source link

'str' object has no attribute "__stored_args__" (ISSUE #7) #10

Closed lohithmunakala closed 4 years ago

lohithmunakala commented 4 years ago

I opened an issue (#7) on 03/09/2020. The problem and the solution are mentioned below with the colab files as well.

On 03/09/2020, a commit was made in fastai/fastcore/uitls.py (fastai/fastcore@ea1c33f) (line 86) which made sure that we need not use "self" when we call the method store_attr. ( separate the names using commas)

I made a small change in the code ie. in the upit/train/cyclegan.py such that it follows the changed fastcore/utils.py structure. I removed the self component for store_attr in cyclegan.py.

The error can be seen here

Screenshot from 2020-09-04 22-27-26

The colab link: https://colab.research.google.com/drive/1lbXhX-bWvTsQcLb2UUFI0UkRpvx_WK2Y?usp=sharing

After the change, the error does not occur as shown here.

Screenshot from 2020-09-04 22-50-17 The Colab link: https://colab.research.google.com/drive/13rrNLBgDulgeHclovxJNtQVbVYcGMKm0?usp=sharing

lohithmunakala commented 4 years ago

Keep getting the error again and again. The error is as follows. Screenshot from 2020-09-04 23-59-01

tmabraham commented 4 years ago

Thank you for discovering and tracking down this error. The fastcore change was just announced here as well.

lohithmunakala commented 4 years ago

Sure thing !

tmabraham commented 4 years ago

Note that the typical workflow for this project (and nbdev projects in general) is to edit the notebooks, run nbdev_build_lib (build module files) and run nbdev_test_nbs (run tests) both from the command line(optionally run nbdev_build_docs if there are documentation changes). The CI error is because you edited the module file and there is difference between the module and the notebook.

Since it's a small bug that you have fixed, you can simply run nbdev_update_lib from the command line to update the notebooks so the notebooks and library match. Please commit these changes, check that the CI passes, and then I can merge

In the future, please edit the notebooks, update the library, and test the notebooks before submitting a PI. Admittedly, it's also my fault that this wasn't very clear in the README.md or the CONTRIBUTING.md files, so I will update those docs accordingly.

lohithmunakala commented 4 years ago

Have made the necessary changes. Thanks for the update (https://github.com/tmabraham/UPIT/pull/10#issuecomment-687557014). The checks have passed now.

tmabraham commented 4 years ago

@lohithmunakala Merged! Thanks again for the fix!

lohithmunakala commented 4 years ago

Closed the issue.