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

Delete parent modules from sys.modules #158

Merged
merged 2 commits into from Jul 24, 2016

Conversation

@atodorov
Copy link
Contributor

commented Jul 20, 2016

I believe this fixes my comment at
#157 (comment)

Here is the test log w/o the fix and then with the fix:

https://travis-ci.org/MrSenko/cosmic-ray/jobs/146035928

https://travis-ci.org/MrSenko/cosmic-ray/jobs/146038003

@atodorov atodorov force-pushed the MrSenko:import_fix branch from 26bdab3 to c7ab6a4 Jul 20, 2016

@atodorov

This comment has been minimized.

Copy link
Contributor Author

commented Jul 20, 2016

FYI this also fixes my problem in #157 where I modify sys.path and load modules via importlib.import_module, at least in the production SUT, but not in the example. I guess in the larger piece of software removing the parent modules from sys.modules is causing them to reload the children and I get the mutated ast loaded. However the same thing doesn't happen with the initial example I've provided.

for parent in module_name.split('.'):
module_to_delete = ("%s.%s" % (module_to_delete, parent)).lstrip('.')
if module_to_delete in sys.modules:
del sys.modules[module_to_delete]

This comment has been minimized.

Copy link
@abingham

abingham Jul 20, 2016

Contributor

This is a bit awkward and old-fashioned. I'd prefer something a bit more modern and (to me, at least) readable:

del_mods = set(accumulate(module_name.split('.'), '{}.{}'.format))
sys.modules = {k: v for k, v in sys.modules.items() if k not in del_mods}

(NB: I haven't actually tested this, just sketched it out.)

This comment has been minimized.

Copy link
@atodorov

atodorov Jul 20, 2016

Author Contributor

For some reason the assignment to sys.modules doesn't have the desired effect. Looping through del_mods and deleting them one by one however seems to be fine.

This comment has been minimized.

Copy link
@abingham

abingham Jul 21, 2016

Contributor

Interesting. Something must be holding a reference to the original.

On Wed, Jul 20, 2016, 12:31 Alexander Todorov notifications@github.com
wrote:

In cosmic_ray/importing.py
#158 (comment):

@@ -61,8 +61,11 @@ def using_mutant(module_name, mutant):
mutated AST.

 """
  • if module_name in sys.modules:
  •    del sys.modules[module_name]
    
  • module_to_delete = ""
  • for parent in module_name.split('.'):
  •    module_to_delete = ("%s.%s" % (module_to_delete, parent)).lstrip('.')
    
  •    if module_to_delete in sys.modules:
    
  •        del sys.modules[module_to_delete]
    

For some reason the assignment to sys.modules doesn't have the desired
effect. Looping through del_mods and deleting them one by one however seems
to be fine.


You are receiving this because you commented.

Reply to this email directly, view it on GitHub
https://github.com/sixty-north/cosmic-ray/pull/158/files/c7ab6a4cad1cda12495a235d173c8c3f2cf2b610#r71501432,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAE1eC9cOAD8eEsEdNYlnCksmlE8Vj2Kks5qXfjngaJpZM4JQjEd
.

@atodorov atodorov force-pushed the MrSenko:import_fix branch 2 times, most recently from d1695b5 to 75b64b5 Jul 20, 2016

@atodorov atodorov force-pushed the MrSenko:import_fix branch from 75b64b5 to 3337617 Jul 20, 2016

@abingham abingham merged commit 3337617 into sixty-north:master Jul 24, 2016

2 checks passed

code-quality/landscape Code quality check completed successfully
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@abingham

This comment has been minimized.

Copy link
Contributor

commented Jul 24, 2016

I'll admit that I don't entirely understand the underlying issue here - in particular, why from sandwich.ham import ham doesn't invoke the finder when sandwich.ham.ham isn't in sys.modules - but this patch does seem to address at least part of the issue.

@atodorov atodorov deleted the MrSenko:import_fix branch Oct 5, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.