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

Add news attribute to TextChannel #1370

Merged
merged 2 commits into from
May 20, 2022

Conversation

NeloBlivion
Copy link
Member

Add the news attribute to TextChannel which implements is_news(), which in turn fixes the AttributeError raised in the base _TextChannel.__repr__ function.

Checklist

  • If code changes were made then they have been tested.
    • I have updated the documentation to reflect the changes.
  • If type: ignore comments were used, a comment is also left explaining why
  • 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, typehinting, examples, ...)

Add the `news` attribute to TextChannel which implements `is_news()`, which in turn fixes the base `_TextChannel.__repr__` function.
@NeloBlivion
Copy link
Member Author

The AttributeError referenced

Traceback (most recent call last):
  File "/home/container/.local/lib/python3.10/site-packages/discord/channel.py", line 190, in __repr__
    attrs = [(val, getattr(self, val)) for val in self._repr_attrs]
  File "/home/container/.local/lib/python3.10/site-packages/discord/channel.py", line 190, in <listcomp>
    attrs = [(val, getattr(self, val)) for val in self._repr_attrs]
AttributeError: 'TextChannel' object has no attribute 'news'

I'm not entirely sure if the approach with this change is appropriate; this is most comparable to is_nsfw() and nsfw existing alongside each other, however nsfw is an attribute that is_nsfw() returns, whereas this is the opposite case. The other options here are removing "news" from _repr_attrs, or appending it manually inside the __repr__ function.

@plun1331
Copy link
Member

I would rather remove the news attr from _repr_attrs, as it doesn't really serve a purpose.
I'm not sure why is_nsfw ever existed, but removing it would be a breaking change.

@Dorukyum
Copy link
Member

How about we add is_news to _repr_attrs instead?

@NeloBlivion
Copy link
Member Author

NeloBlivion commented May 19, 2022

How about we add is_news to _repr_attrs instead?

Just tried... you hit a max recursion depth error. For reference, this is how it's implemented in rc1

def __repr__(self) -> str:
    attrs = [(val, getattr(self, val)) for val in self._repr_attrs]
    joined = " ".join("%s=%r" % t for t in attrs)
    return f"<{self.__class__.__name__} {joined}>"

Whereas in b7, it was manually mapped

attrs = [
    ...
    ("news", self.is_news()),
    ... 
]

is_news isn't an attribute so trying this is guaranteed to fail. Though, I notice only TextChannel was updated to use this new method; all the other classes still manually map the attributes or just return the string without joining a list. Is there any reason for this?

@plun1331 plun1331 added the bug Something isn't working label May 19, 2022
@Lulalaby Lulalaby merged commit 2e580d2 into Pycord-Development:master May 20, 2022
Lulalaby pushed a commit that referenced this pull request Jun 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Status: No status
Development

Successfully merging this pull request may close these issues.

4 participants