Skip to content

Fix pool.map so that order of results is retained#7

Open
emef wants to merge 2 commits intoshazow:masterfrom
emef:master
Open

Fix pool.map so that order of results is retained#7
emef wants to merge 2 commits intoshazow:masterfrom
emef:master

Conversation

@emef
Copy link
Copy Markdown

@emef emef commented Jun 11, 2013

There is no guarantee in which order the threads will complete their task, so the map jobs must know the index their result should have in the final results list.

Also fixed the tests so that nosetests doesn't hang on a test failure (pool not properly shutdown)

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

What's the benefit of this addition?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Test failures raise an exception in the main thread, and do not shut the pools down. On my machine, this hangs up the entire test suite. By using self.pool() to create pools, we get automatic shutdowns regardless of test pass/failure.

Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

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

Fair enough, good catch! Can we change def pool(...) to def get_pool(...) please? Or maybe get_workerpool. Otherwise looks good.

@shazow
Copy link
Copy Markdown
Owner

shazow commented Jun 11, 2013

@jtriley Any thoughts on this before I merge?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants