-
-
Notifications
You must be signed in to change notification settings - Fork 561
fix: use .get() for optional fields in User.__init__ and Client.request #418
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -88,40 +88,40 @@ class User: | |||||||||||||
|
|
||||||||||||||
| def __init__(self, client: Client, data: dict) -> None: | ||||||||||||||
| self._client = client | ||||||||||||||
| legacy = data['legacy'] | ||||||||||||||
| legacy = data.get('legacy', {}) | ||||||||||||||
|
|
||||||||||||||
| self.id: str = data['rest_id'] | ||||||||||||||
| self.created_at: str = legacy['created_at'] | ||||||||||||||
| self.name: str = legacy['name'] | ||||||||||||||
| self.screen_name: str = legacy['screen_name'] | ||||||||||||||
| self.profile_image_url: str = legacy['profile_image_url_https'] | ||||||||||||||
| self.id: str = data.get('rest_id', '') | ||||||||||||||
| self.created_at: str = legacy.get('created_at', '') | ||||||||||||||
| self.name: str = legacy.get('name', '') | ||||||||||||||
| self.screen_name: str = legacy.get('screen_name', '') | ||||||||||||||
| self.profile_image_url: str = legacy.get('profile_image_url_https', '') | ||||||||||||||
| self.profile_banner_url: str = legacy.get('profile_banner_url') | ||||||||||||||
| self.url: str = legacy.get('url') | ||||||||||||||
| self.location: str = legacy['location'] | ||||||||||||||
| self.description: str = legacy['description'] | ||||||||||||||
| self.description_urls: list = legacy['entities']['description']['urls'] | ||||||||||||||
| self.urls: list = legacy['entities'].get('url', {}).get('urls') | ||||||||||||||
| self.pinned_tweet_ids: list[str] = legacy['pinned_tweet_ids_str'] | ||||||||||||||
| self.is_blue_verified: bool = data['is_blue_verified'] | ||||||||||||||
| self.verified: bool = legacy['verified'] | ||||||||||||||
| self.possibly_sensitive: bool = legacy['possibly_sensitive'] | ||||||||||||||
| self.can_dm: bool = legacy['can_dm'] | ||||||||||||||
| self.can_media_tag: bool = legacy['can_media_tag'] | ||||||||||||||
| self.want_retweets: bool = legacy['want_retweets'] | ||||||||||||||
| self.default_profile: bool = legacy['default_profile'] | ||||||||||||||
| self.default_profile_image: bool = legacy['default_profile_image'] | ||||||||||||||
| self.has_custom_timelines: bool = legacy['has_custom_timelines'] | ||||||||||||||
| self.followers_count: int = legacy['followers_count'] | ||||||||||||||
| self.fast_followers_count: int = legacy['fast_followers_count'] | ||||||||||||||
| self.normal_followers_count: int = legacy['normal_followers_count'] | ||||||||||||||
| self.following_count: int = legacy['friends_count'] | ||||||||||||||
| self.favourites_count: int = legacy['favourites_count'] | ||||||||||||||
| self.listed_count: int = legacy['listed_count'] | ||||||||||||||
| self.media_count = legacy['media_count'] | ||||||||||||||
| self.statuses_count: int = legacy['statuses_count'] | ||||||||||||||
| self.is_translator: bool = legacy['is_translator'] | ||||||||||||||
| self.translator_type: str = legacy['translator_type'] | ||||||||||||||
| self.withheld_in_countries: list[str] = legacy['withheld_in_countries'] | ||||||||||||||
| self.location: str = legacy.get('location', '') | ||||||||||||||
| self.description: str = legacy.get('description', '') | ||||||||||||||
| self.description_urls: list = legacy.get('entities', {}).get('description', {}).get('urls', []) | ||||||||||||||
| self.urls: list = legacy.get('entities', {}).get('url', {}).get('urls') | ||||||||||||||
| self.pinned_tweet_ids: list[str] = legacy.get('pinned_tweet_ids_str', []) | ||||||||||||||
|
Comment on lines
+102
to
+104
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. suggestion: Align
Suggested change
|
||||||||||||||
| self.is_blue_verified: bool = data.get('is_blue_verified', False) | ||||||||||||||
| self.verified: bool = legacy.get('verified', False) | ||||||||||||||
| self.possibly_sensitive: bool = legacy.get('possibly_sensitive', False) | ||||||||||||||
| self.can_dm: bool = legacy.get('can_dm', False) | ||||||||||||||
| self.can_media_tag: bool = legacy.get('can_media_tag', False) | ||||||||||||||
| self.want_retweets: bool = legacy.get('want_retweets', False) | ||||||||||||||
| self.default_profile: bool = legacy.get('default_profile', False) | ||||||||||||||
| self.default_profile_image: bool = legacy.get('default_profile_image', False) | ||||||||||||||
| self.has_custom_timelines: bool = legacy.get('has_custom_timelines', False) | ||||||||||||||
| self.followers_count: int = legacy.get('followers_count', 0) | ||||||||||||||
| self.fast_followers_count: int = legacy.get('fast_followers_count', 0) | ||||||||||||||
| self.normal_followers_count: int = legacy.get('normal_followers_count', 0) | ||||||||||||||
| self.following_count: int = legacy.get('friends_count', 0) | ||||||||||||||
| self.favourites_count: int = legacy.get('favourites_count', 0) | ||||||||||||||
| self.listed_count: int = legacy.get('listed_count', 0) | ||||||||||||||
| self.media_count = legacy.get('media_count', 0) | ||||||||||||||
| self.statuses_count: int = legacy.get('statuses_count', 0) | ||||||||||||||
| self.is_translator: bool = legacy.get('is_translator', False) | ||||||||||||||
| self.translator_type: str = legacy.get('translator_type', '') | ||||||||||||||
| self.withheld_in_countries: list[str] = legacy.get('withheld_in_countries', []) | ||||||||||||||
| self.protected: bool = legacy.get('protected', False) | ||||||||||||||
|
|
||||||||||||||
| @property | ||||||||||||||
|
|
||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Chained
.get(..., {})still breaks when intermediate values are explicitlyNone.dict.get(key, default)only returnsdefaultwhen the key is missing; it returnsNoneif the key is present with aNonevalue. Looking atbuild_user_dataintwikit/utils.py(used byfollow_user/block_user/mute_user/etc.),legacy['entities']is set viaraw_data.get('entities'), so it can beNone. In that case:This re-introduces the same class of crash the PR is trying to eliminate. Same concern applies to
entities.descriptionandentities.urlbeingNone.Also, there is an inconsistency on Line 103:
self.urlsdefaults toNonewhile the docstring/type annotation sayslist, andself.description_urlson Line 102 defaults to[]. Prefer[]for both to keep the attribute type stable for downstream consumers.🛡️ Proposed fix
The same
or {}pattern also covers thedata.get('legacy', {})on Line 91 beingNoneif you want to harden that further:📝 Committable suggestion
🤖 Prompt for AI Agents