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

247 is alive completion #283

Merged
merged 4 commits into from
Jan 30, 2020
Merged

247 is alive completion #283

merged 4 commits into from
Jan 30, 2020

Conversation

mcepl
Copy link
Contributor

@mcepl mcepl commented Jan 13, 2020

This is an attempt to fix #247, but still unsuccesful.

@soupytwist , any idea, what's going on? I have tried to replace threading.Thead.isAlive (which has been deprecated and unnecessary since 2.6) with .join (which I guess will never be deprecated), but still doesn’t seem to work. Why rope doesn’t suggest it?

@mcepl
Copy link
Contributor Author

mcepl commented Jan 13, 2020

@gwelymernans ^^^

@mcepl mcepl changed the title WIP: 247 is alive completion 247 is alive completion Jan 13, 2020
@soupytwist
Copy link
Contributor

I can't explain this, but I found a workaround for this issue which might hint at the reason. collections.Iterable and collections.Iterator are deprecated and now alias collections.abc.Iterable and collections.abc.Iterator, respectively. The former two are removed in 3.8.

I don't really know what's happening, but I found that adding these two lines to the else in type_hinting/utils.py makes the tests pass.

     else:
         mod_name, attr_name = type_name.rsplit('.', 1)
+        if mod_name == 'collections' and attr_name in ('Iterable', 'Iterator'):
+            mod_name = 'collections.abc'
         try:

I'm not suggesting that as a patch, but it might help give you some idea what's going on. If not... it's a potential ugly workaround. There is probably an underlying issue here that will affect similar cases, though, which should be fixed.

Hope it helps...

@mcepl
Copy link
Contributor Author

mcepl commented Jan 14, 2020

OK, so #247 is truly fixed, but now we have another problem with Python 3.8. Let us split this PR into two.

The lesson is simple exception Exception: pass is ALWAYS The Wrong Thing™ to do.

@mcepl mcepl requested a review from soupytwist January 14, 2020 14:25
bmwiedemann added a commit to bmwiedemann/openSUSE that referenced this pull request Jan 14, 2020
@mcepl mcepl mentioned this pull request Jan 30, 2020
Copy link

@lucomsky lucomsky left a comment

Choose a reason for hiding this comment

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

I ran through all changes and think they are good. Can't say more deep comment in the lack of experience in rope project development.

@mcepl mcepl merged commit 67aeb33 into python-rope:master Jan 30, 2020
@mcepl mcepl deleted the 247_isAlive-completion branch January 30, 2020 15:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants