Skip to content

Conversation

nchristensen
Copy link
Contributor

@nchristensen nchristensen commented Nov 21, 2022

This adds a concurrent.futures.Executor implementation for Charm4py so Charm4py can be dropped in wherever ThreadPoolExecutor, ProcessPoolExecutor, MPI4pyPoolExecutor, etc. is used.

A PoolExecutor class in pool.py implements the Executor API by calling to equivalent Pool methods. This pull request also makes charm4py.threads.Future inherit from concurrent.futures.Future and implements its API.

TODO:

  • Fix Future.set_running_or_notify_cancel() and figure out where to call it. Execution currently hangs without explicitly calling Future.get().
  • Update dependencies
  • Update documentation
  • Add tests
  • Increment Charm4py version
  • Add _max_workers property to Executor
@lgtm-com
Copy link

lgtm-com bot commented Nov 21, 2022

This pull request introduces 2 alerts when merging 1b08833 into 74791f9 - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method
  • 1 for First parameter of a method is not named 'self'

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link

lgtm-com bot commented Nov 21, 2022

This pull request introduces 2 alerts when merging 08cf569 into 74791f9 - view on LGTM.com

new alerts:

  • 1 for Signature mismatch in overriding method
  • 1 for First parameter of a method is not named 'self'

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@ZwFink
Copy link
Contributor

ZwFink commented Nov 21, 2022

Hi Nick,
This is some really cool work! I can help out later this week or early next week to help with testing and documentation.

@nchristensen
Copy link
Contributor Author

Cool, that would be great! It could definitely use another pair of eyes.

@lgtm-com
Copy link

lgtm-com bot commented Nov 22, 2022

This pull request fixes 1 alert when merging 9822860 into 74791f9 - view on LGTM.com

fixed alerts:

  • 1 for Illegal raise

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link

lgtm-com bot commented Dec 6, 2022

This pull request fixes 1 alert when merging 282a11b into 74791f9 - view on LGTM.com

fixed alerts:

  • 1 for Illegal raise

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link

lgtm-com bot commented Dec 6, 2022

This pull request fixes 1 alert when merging b71e4b3 into 74791f9 - view on LGTM.com

fixed alerts:

  • 1 for Illegal raise

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link

lgtm-com bot commented Dec 7, 2022

This pull request introduces 7 alerts and fixes 2 when merging 081d3dc into 74791f9 - view on LGTM.com

new alerts:

  • 5 for Unused import
  • 2 for Unreachable code

fixed alerts:

  • 1 for Missing call to `__init__` during object initialization
  • 1 for Illegal raise

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@lgtm-com
Copy link

lgtm-com bot commented Dec 7, 2022

This pull request introduces 7 alerts and fixes 2 when merging 2bbb9a8 into 74791f9 - view on LGTM.com

new alerts:

  • 5 for Unused import
  • 2 for Unreachable code

fixed alerts:

  • 1 for Missing call to `__init__` during object initialization
  • 1 for Illegal raise

Heads-up: LGTM.com's PR analysis will be disabled on the 5th of December, and LGTM.com will be shut down ⏻ completely on the 16th of December 2022. Please enable GitHub code scanning, which uses the same CodeQL engine ⚙️ that powers LGTM.com. For more information, please check out our post on the GitHub blog.

@nchristensen nchristensen marked this pull request as draft December 8, 2022 00:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

2 participants