Skip to content

Converting "member" to "type(member)" on line 311 of completion.py to fix issue 272#290

Closed
bradleydamato wants to merge 4 commits intogoogle:masterfrom
bradleydamato:Issue272Fix
Closed

Converting "member" to "type(member)" on line 311 of completion.py to fix issue 272#290
bradleydamato wants to merge 4 commits intogoogle:masterfrom
bradleydamato:Issue272Fix

Conversation

@bradleydamato
Copy link
Copy Markdown
Contributor

@bradleydamato bradleydamato commented Oct 2, 2020

Passes:
completion_test.py
core_test.py
main_test.py

@google-cla google-cla bot added the cla: yes Author has signed CLA label Oct 2, 2020
@bradleydamato
Copy link
Copy Markdown
Contributor Author

bradleydamato commented Oct 2, 2020

@dbieber here is the PR for Issue 272 #272

@bradleydamato bradleydamato mentioned this pull request Oct 2, 2020
Closed
dbieber
dbieber reviewed Oct 2, 2020
View reviewed changes
fire/completion.py Outdated
if verbose:
return True
if member in (absolute_import, division, print_function):
if type(member) in (absolute_import, division, print_function):
Copy link
Copy Markdown
Collaborator

@dbieber dbieber Oct 2, 2020 •
edited
Loading

Choose a reason for hiding this comment

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

I don't think this is quite the check we want.

In Python 3 type(absolute_import) is __future__._Feature, which is fine.
However in Python 2 type(absolute_import) is instance which is too broad.

Instead we can do if (member is absolute_import or member is division or member is print_function)

Copy link
Copy Markdown
Contributor Author

@bradleydamato bradleydamato Oct 2, 2020

Choose a reason for hiding this comment

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

Sounds good @dbieber I'll make the change and push again

@bradleydamato
Copy link
Copy Markdown
Contributor Author

bradleydamato commented Oct 2, 2020

Looks like the line is too long; third time is the charm :). BRB.

@dbieber
Copy link
Copy Markdown
Collaborator

dbieber commented Oct 2, 2020

Looks good, thanks for the PR.

dbieber pushed a commit that referenced this pull request Oct 2, 2020
@bradleydamato @dbieber
Change member future check to fix issue 272
c378906 
bb58f26 by bradleydamato <[email protected]>
ebc446f by bradleydamato <[email protected]>
98b131f by bradleydamato <[email protected]>

COPYBARA_INTEGRATE_REVIEW=#290 from bradleydamato:Issue272Fix 98b131f
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
@dbieber
Copy link
Copy Markdown
Collaborator

dbieber commented Oct 2, 2020

Merged in c378906

@dbieber dbieber closed this Oct 2, 2020
CherylMoonba added a commit to CherylMoonba/devices that referenced this pull request Jul 20, 2025
@CherylMoonba
Change member future check to fix issue 272
d303d95 
bb58f2611469851d842f95a65da1bf926c0c85c3 by bradleydamato <[email protected]>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <[email protected]>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <[email protected]>

COPYBARA_INTEGRATE_REVIEW=google/python-fire#290 from bradleydamato:Issue272Fix 98b131f15d2f9540628942f975699a00a89d0c87
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
KurtMuelle added a commit to KurtMuelle/obtained that referenced this pull request Jul 22, 2025
@KurtMuelle
Change member future check to fix issue 272
da5815e 
bb58f2611469851d842f95a65da1bf926c0c85c3 by bradleydamato <[email protected]>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <[email protected]>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <[email protected]>

COPYBARA_INTEGRATE_REVIEW=google/python-fire#290 from bradleydamato:Issue272Fix 98b131f15d2f9540628942f975699a00a89d0c87
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
hwlegends pushed a commit to hwlegends/python-fire that referenced this pull request Aug 20, 2025
@bradleydamato @dbieber
Change member future check to fix issue 272
dd7aead 
bb58f2611469851d842f95a65da1bf926c0c85c3 by bradleydamato <[email protected]>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <[email protected]>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <[email protected]>

COPYBARA_INTEGRATE_REVIEW=google/python-fire#290 from bradleydamato:Issue272Fix 98b131f15d2f9540628942f975699a00a89d0c87
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
Hernandez19891 added a commit to Hernandez19891/Illumina that referenced this pull request Sep 15, 2025
@Hernandez19891
Change member future check to fix issue 272
db8678d 
bb58f2611469851d842f95a65da1bf926c0c85c3 by bradleydamato <[email protected]>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <[email protected]>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <[email protected]>

COPYBARA_INTEGRATE_REVIEW=google/python-fire#290 from bradleydamato:Issue272Fix 98b131f15d2f9540628942f975699a00a89d0c87
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dbieber dbieber dbieber left review comments

Assignees

No one assigned

Labels

cla: yes Author has signed CLA

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@bradleydamato @dbieber