Fix TypeError when setting Embed.thumbnail and Embed.image property #41

Merged
null2264 merged 3 commits from 2.0 into 2.0 2021-09-02 19:46:56 +00:00
null2264 commented 2021-09-01 12:55:42 +00:00 (Migrated from github.com)

Summary

Fixed TypeError: image() takes 1 positional argument but 2 were given

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • This PR fixes an issue.
  • This PR adds something new (e.g. new method or parameters).
  • This PR is a breaking change (e.g. methods or parameters removed/renamed)
  • This PR is not a code change (e.g. documentation, README, ...)
## Summary <!-- What is this pull request for? Does it fix any issues? --> Fixed `TypeError: image() takes 1 positional argument but 2 were given` ## Checklist <!-- Put an x inside [ ] to check it, like so: [x] --> - [x] If code changes were made then they have been tested. - [ ] I have updated the documentation to reflect the changes. - [x] This PR fixes an issue. - [ ] This PR adds something new (e.g. new method or parameters). - [ ] This PR is a breaking change (e.g. methods or parameters removed/renamed) - [ ] This PR is **not** a code change (e.g. documentation, README, ...)
isaa-ctaylor commented 2021-09-01 12:57:11 +00:00 (Migrated from github.com)

You are just supposed to do url=

You are just supposed to do `url=`
null2264 commented 2021-09-01 12:58:34 +00:00 (Migrated from github.com)

But i did? My code is literally e.set_image(url=avatar.with_size(1024).url)

But i did? My code is literally `e.set_image(url=avatar.with_size(1024).url)`
isaa-ctaylor commented 2021-09-01 12:59:42 +00:00 (Migrated from github.com)

Could you show the code? From where you define e?

Could you show the code? From where you define `e`?
null2264 commented 2021-09-01 13:04:57 +00:00 (Migrated from github.com)

I think that's unrelated but here is the full code

e = discord.Embed(
        title="{}'s Avatar".format(user.name),
        description=links,
)
e.set_image(url=user.display_avatar.with_size(1024).url)
await ctx.reply(embed=e)
I think that's unrelated but here is the full code ```py e = discord.Embed( title="{}'s Avatar".format(user.name), description=links, ) e.set_image(url=user.display_avatar.with_size(1024).url) await ctx.reply(embed=e) ```
isaa-ctaylor commented 2021-09-01 13:07:01 +00:00 (Migrated from github.com)

Could I see the full traceback as well? In the original error you mentioned (TypeError: image() takes 1 positional argument but 2 were given), image() seems to be the culprit, not set_image?

Could I see the full traceback as well? In the original error you mentioned (`TypeError: image() takes 1 positional argument but 2 were given`), `image()` seems to be the culprit, not set_image?
null2264 commented 2021-09-01 13:11:27 +00:00 (Migrated from github.com)

Yea it is image() that's what i changed in this PR,

set_image is just self.image = new_url that runs image() which is a setter for image property, and that setter's url param being keyword only breaks it...

Yea it is `image()` that's what i changed in this PR, `set_image` is just `self.image = new_url` that runs `image()` which is a setter for `image` property, and that setter's `url` param being keyword only breaks it...
isaa-ctaylor commented 2021-09-01 13:15:57 +00:00 (Migrated from github.com)

Oh. I failed to look properly at what the code was, my bad, sorry!

Oh. I failed to look properly at what the code was, my bad, sorry!
iDutchy (Migrated from github.com) requested changes 2021-09-02 01:13:55 +00:00
iDutchy (Migrated from github.com) left a comment

It has been brought to my attention that both image.setter and thumbnail.setter actually do not work if you later want to remove them. Changing to

    @image.setter
    def image(self: E, *, url: Any):
        if url is EmptyEmbed:
            del self._image
        else:
            self._image = {
                'url': str(url),
            }
   and same for thumbnail should fix that
It has been brought to my attention that both image.setter and thumbnail.setter actually do not work if you later want to remove them. Changing to ``` @image.setter def image(self: E, *, url: Any): if url is EmptyEmbed: del self._image else: self._image = { 'url': str(url), } ``` and same for thumbnail should fix that
iDutchy (Migrated from github.com) approved these changes 2021-09-02 02:35:24 +00:00
iDutchy commented 2021-09-02 02:36:42 +00:00 (Migrated from github.com)

Looks good. There's only a stray return in the thumbnail.setter which can be removed. Also may need a .. versionchanged:: 2.0 to add that image/thumbnail can now be set this way?

Looks good. There's only a stray `return` in the thumbnail.setter which can be removed. Also may need a `.. versionchanged:: 2.0` to add that image/thumbnail can now be set this way?
null2264 commented 2021-09-02 05:07:33 +00:00 (Migrated from github.com)

Also may need a .. versionchanged:: 2.0 to add that image/thumbnail can now be set this way?

Like this?

.. versionchanged:: 2.0
    Can now be set without using `set_image()`

I'm not good at wording and stuff so...

> Also may need a .. versionchanged:: 2.0 to add that image/thumbnail can now be set this way? Like this? ``` .. versionchanged:: 2.0 Can now be set without using `set_image()` ``` I'm not good at wording and stuff so...
IAmTomahawkx commented 2021-09-02 19:46:50 +00:00 (Migrated from github.com)

Thanks

Thanks
Sign in to join this conversation.
No description provided.