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

Add shortcuts to imports of datastores, eventbrokers, executors, serializers, and triggers #1007

Open
1 task done
HomerusJa opened this issue Dec 26, 2024 · 12 comments
Open
1 task done

Comments

@HomerusJa
Copy link

Things to check first

  • I have searched the existing issues and didn't find my feature already requested there

Feature description

A reorganization of the package's import structure by consolidating related classes in their respective init.py files. This would maintain the one-class-per-file architecture while exposing the classes at a higher package level. The implementation would involve:

  • Moving class imports into the corresponding package's init.py file
  • Maintaining the current file structure underneath
  • Providing direct access to classes from the package level rather than requiring deep module imports

Use case

Consider configuring APScheduler in a Python application. Currently, you need verbose imports from deep module paths:

from apscheduler.datastores.mongodb import MongoDBDataStore
from apscheduler.executors.async_ import AsyncJobExecutor
from apscheduler.executors.subprocess import ProcessPoolJobExecutor
from apscheduler.executors.threadpool import ThreadPoolJobExecutor

With the proposed feature, imports become more concise and intuitive:

from apscheduler.datastores import MongoDBDataStore
from apscheduler.executors import AsyncJobExecutor, ProcessPoolJobExecutor, ThreadPoolJobExecutor

This eliminates redundant path components (like .mongodb import MongoDBDataStore) while maintaining the same functionality and code organization.

@HomerusJa
Copy link
Author

Can this go into v4.0 as it is really easy to implement, and for me it really is a pain point when using apscheduler.

@agronholm
Copy link
Owner

No, I am not interested in a structure where every data store and executor implementation has to be imported when you import one. What would even happen when it tries to import optional depedencies?

@agronholm
Copy link
Owner

Besides, why aren't you just using autocomplete like everybody else?

@HomerusJa
Copy link
Author

I want to address the optional dependencies concern you raised - you were absolutely right that I should have considered this more carefully. After researching how other libraries handle this, I found several elegant solutions. Here's one approach that would require minimal code changes while providing clear feedback to users:

class DependencyNotFound:
    """
    A proxy class that raises helpful ImportError messages when any attribute
    is accessed or the class is called.
    """
    def __init__(self, dependency_name: str, group_name: str | None = None) -> None:
        self.dependency_name = dependency_name
        self.group_name = group_name
        if group_name is not None:
            self._error_msg = (
                f"{dependency_name} is not installed. "
                f"Please install APScheduler with the '{group_name}' extra: "
                f"pip install 'apscheduler[{group_name}]'"
            )
        else:
            self._error_msg = (
                f"{dependency_name} is not installed. "
                f"Please install it directly: "
                f"pip install {dependency_name}"
            )
    
    def __getattr__(self, _: str) -> Any:
        raise ImportError(self._error_msg)
    
    def __call__(self, *args: Any, **kwargs: Any) -> Any:
        raise ImportError(self._error_msg)
    
    def __repr__(self) -> str:
        return f"<NotInstalled '{self.dependency_name}'>"

# Usage in __init__.py would be simple:
try:
    from apscheduler.datastores.mongodb import MongoDBDataStore
except ImportError:
    MongoDBDataStore = DependencyNotFound("pymongo", "mongodb")

# Or for dependencies that aren't part of a group:
try:
    import some_direct_dependency
except ImportError:
    some_direct_dependency = DependencyNotFound("some-direct-dependency")

This would keep imports clean while providing explicit feedback when optional dependencies are missing. That said, I completely understand if this isn't something you want to include in v4.0. This is clearly a nice-to-have feature rather than a critical one, and I should have framed it that way from the start.

This is my first open source contribution experience, and I'm learning a lot about both the technical considerations and how to be a constructive community member. I appreciate the feedback you've provided - it pushed me to think through the implications more thoroughly. If you're open to it, I'd be happy to implement this approach, including tests, whenever it aligns with the project's timeline - whether that's in v4.0, v4.1, or even later. If not, I completely understand, and I'll focus my future contributions on areas that better align with the project's current needs.​​​​​​​​​​​​​​​​

@agronholm
Copy link
Owner

agronholm commented Dec 27, 2024

I'm still not convinced that this is a good idea. If the only reason for this is to allow someone to do less typing, it's not worth doing. You didn't address my rebuttal about autocomplete yet. See the attached screenshot on how I use data stores (and how you can too).

Nayttotallennevideo.-.2024-12-27.14-30-15.mp4

@HomerusJa
Copy link
Author

First of all, I’ve never seen that autoimporting thing you are doing there. That looks great! How is that possible?

Following some other reasons why I think importing into the ˋ__init__.py` would be a great idea:

  1. Clearer definition of an interface
    I.e. the MongoDB file contains various other internal things, like a custom encoder, several marshaling helpers or the asynchronous cursor, which should not be used by the user. You could solve this by using ˋ__all__ˋ, however, I think by importing into the ˋ__init__.py` this would be cleaner and clearer
  2. Better error messages
    Currently, an import error is thrown when the optional dependency is imported. While this is descriptive, I it doesn’t promote using the dependency groups. Currently, this is only done by the documentation.

@agronholm
Copy link
Owner

First of all, I’ve never seen that autoimporting thing you are doing there. That looks great! How is that possible?

What IDE/Editor are you using? I'm using PyCharm.

I.e. the MongoDB file contains various other internal things, like a custom encoder, several marshaling helpers or the asynchronous cursor, which should not be used by the user. You could solve this by using ˋ__all__ˋ, however, I think by importing into the ˋ__init__.py` this would be cleaner and clearer

The "definition of an interface" is in the documentation, where none of the those things have been mentioned. Furthermore, I consider __all__ a bad pattern to use, as it was originally meant to facilitate wildcard imports which are considered to be bad practice. For re-exporting, the pattern from foo import Bar as Bar is the proper pattern recognized by type checkers.

@HomerusJa
Copy link
Author

What IDE/Editor are you using? I'm using PyCharm.

Funnily enough I am using PyCharm too. Maybe I don’t get that feature as I am using the ruff language server… I’ll have to look into that, because that feature looks great.

The "definition of an interface" is in the documentation

I understand your perspective, but I would argue that in today's development landscape, documentation should not serve as the primary source of information. Instead, the IDE plays a central role, offering features like quick documentation popups, method signature hints, and inline docstring displays. When implemented effectively, these tools can replace traditional API documentation by making key information immediately accessible during development.

Given this, I believe the main focus of documentation today should shift towards providing user guides, tutorials, and high-level conceptual overviews rather than detailing every aspect of the API. IDEs can handle API-level details far more effectively.

Consequently, a well-designed package today should optimize for IDE features, making code as self-documenting as possible. Achieving this requires clear, intuitive interfaces and meaningful naming conventions. A crystal-clear interface is no longer just a convenience; it's an essential part of creating a developer-friendly experience.

@agronholm
Copy link
Owner

What IDE/Editor are you using? I'm using PyCharm.

Funnily enough I am using PyCharm too. Maybe I don’t get that feature as I am using the ruff language server… I’ll have to look into that, because that feature looks great.

If you're not using autocomplete, then you're missing out on the arguably most useful feature of the IDE. In that screencast, I just pressed Ctrl+Alt+Space after typing MongoDB. Another form of autocomplete is pressing Alt+Return after you've fully typed a name, and it will then present you a list of potential names to import to your current module.

The "definition of an interface" is in the documentation

I understand your perspective, but I would argue that in today's development landscape, documentation should not serve as the primary source of information. Instead, the IDE plays a central role, offering features like quick documentation popups, method signature hints, and inline docstring displays. When implemented effectively, these tools can replace traditional API documentation by making key information immediately accessible during development.

Given this, I believe the main focus of documentation today should shift towards providing user guides, tutorials, and high-level conceptual overviews rather than detailing every aspect of the API. IDEs can handle API-level details far more effectively.

Consequently, a well-designed package today should optimize for IDE features, making code as self-documenting as possible. Achieving this requires clear, intuitive interfaces and meaningful naming conventions. A crystal-clear interface is no longer just a convenience; it's an essential part of creating a developer-friendly experience.

Yet, I don't see how moving all the data stores and executors to the same package serves this purpose. They're just all located in their respective subpackages.

@HomerusJa
Copy link
Author

HomerusJa commented Dec 27, 2024

If you're not using autocomplete, then you're missing out on the arguably most useful feature of the IDE. In that screencast, I just pressed Ctrl+Alt+Space after typing MongoDB. Another form of autocomplete is pressing Alt+Return after you've fully typed a name, and it will then present you a list of potential names to import to your current module.

Thanks for the advice!

Yet, I don't see how moving all the data stores and executors to the same package serves this purpose. They're just all located in their respective subpackages.

Perhaps I didn’t state myself as clearly as intended. When importing, like that a really clean dropdown would appear with, again, the interface being defined crystal clear. As this is just convenience, I would understand why you don’t want this feature right now, but I think this is a great thing to do when trying to improve the developer experience. Maybe this is also just me.

Also, what do you think about the better error messages? In my opinion, this alone is a reason why such a solution should at least be implemented at some time, again following the reasoning from my message above.


Another thing: Why do you need the imports to look as if they are directly from the package root? Is there any reason, for example in the serialization functionality or is this just cosmetic?

# Re-export imports, so they look like they live directly in this package
value: Any
for value in list(locals().values()):
    if getattr(value, "__module__", "").startswith("apscheduler."):
        value.__module__ = __name__

I am not able to check right now, but doesn’t this break the jump-to-definition feature of some IDEs?

@agronholm
Copy link
Owner

Perhaps I didn’t state myself as clearly as intended. When importing, like that a really clean dropdown would appear with, again, the interface being defined crystal clear.

You're still not stating yourself clearly, as I have no idea what you mean.

Also, what do you think about the better error messages? In my opinion, this alone is a reason why such a solution should at least be implemented at some time, again following the reasoning from my message above.

APScheduler 3.x did in fact have such error messages, and I'm open to readding them in case of a ModuleNotFoundError - that is, if the module clearly isn't there. What I don't want is to obscure any underlying errors causing the import to fail.

Another thing: Why do you need the imports to look as if they are directly from the package root? Is there any reason, for example in the serialization functionality or is this just cosmetic?

The modules whose names are prefixed with an underscore are private. They're made that way to allow me greater freedom to rearrange the code as I see fit. The public interface for the classes and functions re-exported from __init__ is such that they're supposed to be imported from that location, and not the location they're originally defined in. Furthermore, the documentation refers to them using a fully qualified name pointing to where they're re-exported, and the Sphinx extension sphinx-autodoc-typehints won't be able to find the correct references unless this is done. These are just the reasons I came up with off the top of my head.

@HomerusJa
Copy link
Author

You're still not stating yourself clearly, as I have no idea what you mean.

Okay, I currently have no idea of how to state that better, maybe I’ll come back to this if I know how…

APScheduler 3.x did in fact have such error messages, and I'm open to readding them in case of a ModuleNotFoundError - that is, if the module clearly isn't there. What I don't want is to obscure any underlying errors causing the import to fail.

Great point. Should I add something like that when I am back home?

The modules whose names are prefixed with an underscore are private. They're made that way to allow me greater freedom to rearrange the code as I see fit. The public interface for the classes and functions re-exported from init is such that they're supposed to be imported from that location, and not the location they're originally defined in. Furthermore, the documentation refers to them using a fully qualified name pointing to where they're re-exported, and the Sphinx extension sphinx-autodoc-typehints won't be able to find the correct references unless this is done. These are just the reasons I came up with off the top of my head.

Thank you for that explanation! I just had no idea at all why that was there, now I know…

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

No branches or pull requests

2 participants