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

Api exception improvement #1485

Closed
wants to merge 10 commits into from
Closed

Api exception improvement #1485

wants to merge 10 commits into from

Conversation

PythonCoderAS
Copy link
Contributor

@PythonCoderAS PythonCoderAS commented May 4, 2020

The documentation changes to APIException and related items have been separated from the other PR.

Fix #1484

@PythonCoderAS PythonCoderAS added the Documentation Documentation issue or improvement label May 4, 2020
@PythonCoderAS PythonCoderAS added this to the PRAW 7.1 milestone May 4, 2020
Copy link
Member

@bboe bboe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some comments here. I didn't quite do a full review. Something doesn't quite smell right about having two cases of RedditErrorItem.

praw/exceptions.py Show resolved Hide resolved
) -> List[RedditErrorItem]:
"""Covert an exception list into a :class:`.RedditErrorItem` list.

.. warning:: This method is deprecated. Use the private method of the
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think we should recommend using the private method. We could instead say something like this method is deprecated and will be removed without replacement in PRAW 8.0.

@@ -81,6 +142,33 @@ def parse_exception_list(
)
for exception in exceptions
]
for exception in baselist:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Instead of this flag to differentiate between different cases of RedditErrorItem could we instead use different classes with the same interface?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don’t see the need to use different classes, as the only difference is the __repr__ method.

@PythonCoderAS PythonCoderAS modified the milestones: PRAW 7.1, PRAW 7.1.1 Jun 23, 2020
@LilSpazJoekp LilSpazJoekp modified the milestones: PRAW 7.1.1, PRAW 8.0 Feb 18, 2021
@bboe
Copy link
Member

bboe commented May 20, 2021

Author no longer active on this project :'(. Closing PRs.

@bboe bboe closed this May 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation Documentation issue or improvement
Projects
None yet
Development

Successfully merging this pull request may close these issues.

APIException REPR broken when using multiple entries
3 participants