Skip to content

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

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.

Sign up for GitHub

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

Jump to bottom
Closed
bradleydamato wants to merge 4 commits into from
Closed

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

bradleydamato wants to merge 4 commits into from

Conversation

@bradleydamato
Copy link
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
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
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
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
Contributor Author

bradleydamato commented Oct 2, 2020

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

@dbieber
Copy link
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 <bradleydamato@gmail.com>
ebc446f by bradleydamato <bradleydamato@gmail.com>
98b131f by bradleydamato <bradleydamato@gmail.com>

COPYBARA_INTEGRATE_REVIEW=#290 from bradleydamato:Issue272Fix 98b131f
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
@dbieber
Copy link
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 <bradleydamato@gmail.com>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <bradleydamato@gmail.com>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <bradleydamato@gmail.com>

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 <bradleydamato@gmail.com>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <bradleydamato@gmail.com>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <bradleydamato@gmail.com>

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 <bradleydamato@gmail.com>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <bradleydamato@gmail.com>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <bradleydamato@gmail.com>

COPYBARA_INTEGRATE_REVIEW=google/python-fire#290 from bradleydamato:Issue272Fix 98b131f15d2f9540628942f975699a00a89d0c87
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
marsMerge2021 added a commit to marsMerge2021/NTPMeasures that referenced this pull request Aug 31, 2025
@marsMerge2021
Change member future check to fix issue 272
7e2f551 
bb58f2611469851d842f95a65da1bf926c0c85c3 by bradleydamato <bradleydamato@gmail.com>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <bradleydamato@gmail.com>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <bradleydamato@gmail.com>

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 <bradleydamato@gmail.com>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <bradleydamato@gmail.com>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <bradleydamato@gmail.com>

COPYBARA_INTEGRATE_REVIEW=google/python-fire#290 from bradleydamato:Issue272Fix 98b131f15d2f9540628942f975699a00a89d0c87
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
1278827 added a commit to 1278827/Hardware that referenced this pull request Sep 23, 2025
@1278827
Change member future check to fix issue 272
9baf7da 
bb58f2611469851d842f95a65da1bf926c0c85c3 by bradleydamato <bradleydamato@gmail.com>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <bradleydamato@gmail.com>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <bradleydamato@gmail.com>

COPYBARA_INTEGRATE_REVIEW=google/python-fire#290 from bradleydamato:Issue272Fix 98b131f15d2f9540628942f975699a00a89d0c87
PiperOrigin-RevId: 335063289
Change-Id: I1715092509aad11b0bbb681b76a1db7ab48e9a78
VLLanguage added a commit to VLLanguage/VLLanguage that referenced this pull request Nov 23, 2025
@VLLanguage
Change member future check to fix issue 272
95deabc 
bb58f2611469851d842f95a65da1bf926c0c85c3 by bradleydamato <bradleydamato@gmail.com>
ebc446fa79702ce532fb2e0355b69335e6ef027a by bradleydamato <bradleydamato@gmail.com>
98b131f15d2f9540628942f975699a00a89d0c87 by bradleydamato <bradleydamato@gmail.com>

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