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

interface / naming #547

Closed
ltalirz opened this issue Feb 1, 2025 · 1 comment · Fixed by #548
Closed

interface / naming #547

ltalirz opened this issue Feb 1, 2025 · 1 comment · Fixed by #548

Comments

@ltalirz
Copy link

ltalirz commented Feb 1, 2025

Hi,

since I'm reading the documentation of this library for the first time, I thought I would take some notes on the thoughts that pop into my head in the hope they may be useful

  • I think introducing the library via the Executor interface of the Python standard library and the ProcessPoolExecutor/ThreadPoolExecutor is great and very clear
  • Following this introduction (and the name executor lib), I had expected the library to provide a bunch of new executor classes, such as RemotePoolExecutor, ... However, instead of using different classes for different types of executors, as the standard library does, it provides only one Executor class (clashing with the name of the abstract interface of the standard library). This is unexpected for me
  • It seems to me, the Executor class must be doing rather different things depending on the value of the backend variable. Would it not be better (both for the clarity of user source code, for clarity of the documentation and also for the simplicity of the implementation) to have dedicated subclasses for different backends?
  • The documentation could then actually start with an example that uses the standard library ProcessPoolExecutor/ThreadPoolExecutor (which many developers already know how to use), and then go on to show how you can simply use executorlib's classes as a drop-in replacement to get caching / move to remote resources, etc.
@jan-janssen
Copy link
Member

@ltalirz Thanks for taking a look, I really like the idea of separating the different executors. I am going to create a pull request to test this concept and then we can discuss it in more detail.

@jan-janssen jan-janssen linked a pull request Feb 1, 2025 that will close this issue
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 a pull request may close this issue.

2 participants