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

Simplify imports of common classes #957

Closed
simonw opened this issue Aug 29, 2020 · 7 comments
Closed

Simplify imports of common classes #957

simonw opened this issue Aug 29, 2020 · 7 comments

Comments

@simonw
Copy link
Owner

simonw commented Aug 29, 2020

There are only a few classes that plugins need to import. It would be nice if these imports were as short and memorable as possible.

For example:

from datasette.app import Datasette
from datasette.utils.asgi import Response

Could both become:

from datasette import Datasette
from datasette import Response
@simonw
Copy link
Owner Author

simonw commented Aug 29, 2020

Reviewing https://github.com/search?q=user%3Asimonw+%22from+datasette%22&type=Code I spotted these others:

# Various:
from datasette.utils import path_with_replaced_args
from datasette.plugins import pm
from datasette.utils import QueryInterrupted
from datasette.utils.asgi import Response, Forbidden, NotFound

# datasette-publish-vercel:
from datasette.publish.common import (
    add_common_publish_arguments_and_options,
    fail_if_publish_binary_not_installed
)
from datasette.utils import temporary_docker_directory

# datasette-insert
from datasette.utils import actor_matches_allow, sqlite3

# obsolete: russian-ira-facebook-ads-datasette 
from datasette.utils import TableFilter

# simonw/museums
from datasette.utils.asgi import asgi_send

# datasette-media
from datasette.utils.asgi import Response, asgi_send_file

# datasette/tests/plugins/my_plugin.py
from datasette.facets import Facet

# datasette-graphql
from datasette.views.table import TableView

@simonw
Copy link
Owner Author

simonw commented Aug 29, 2020

Of these I think I'm going to promote the following to being importable directly from datasette:

  • from datasette.app import Datasette
  • from datasette.utils import QueryInterrupted
  • from datasette.utils.asgi import Response, Forbidden, NotFound
  • from datasette.utils import actor_matches_allow

All of the rest are infrequently used enough (or clearly named enough) that I'm happy to leave them as-is.

@simonw
Copy link
Owner Author

simonw commented Aug 30, 2020

I tried doing this and got this error:

(datasette) datasette % pytest
==================================================================== test session starts =====================================================================
platform darwin -- Python 3.8.5, pytest-6.0.1, py-1.9.0, pluggy-0.13.1
rootdir: /Users/simon/Dropbox/Development/datasette, configfile: pytest.ini
plugins: asyncio-0.14.0, timeout-1.4.2
collected 1 item / 23 errors                                                                                                                                 

=========================================================================== ERRORS ===========================================================================
_____________________________________________________________ ERROR collecting tests/test_api.py _____________________________________________________________
ImportError while importing test module '/Users/simon/Dropbox/Development/datasette/tests/test_api.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
tests/test_api.py:1: in <module>
    from datasette.plugins import DEFAULT_PLUGINS
datasette/__init__.py:2: in <module>
    from .app import Datasette
datasette/app.py:30: in <module>
    from .views.base import DatasetteError, ureg
datasette/views/base.py:12: in <module>
    from datasette.plugins import pm
datasette/plugins.py:26: in <module>
    mod = importlib.import_module(plugin)
/usr/local/opt/python@3.8/Frameworks/Python.framework/Versions/3.8/lib/python3.8/importlib/__init__.py:127: in import_module
    return _bootstrap._gcd_import(name[level:], package, level)
datasette/publish/heroku.py:2: in <module>
    from datasette import hookimpl
E   ImportError: cannot import name 'hookimpl' from partially initialized module 'datasette' (most likely due to a circular import) (/Users/simon/Dropbox/Development/datasette/datasette/__init__.py)

That's with datasette/__init__.py looking like this:

from datasette.version import __version_info__, __version__  # noqa
from .app import Datasette
from .utils.asgi import Forbidden, NotFound, Response
from .utils import actor_matches_allow, QueryInterrupted
from .hookspecs import hookimpl  # noqa
from .hookspecs import hookspec  # noqa


__all__ = [
    "actor_matches_allow",
    "hookimpl",
    "hookspec",
    "QueryInterrupted",
    "Forbidden",
    "NotFound",
    "Response",
    "Datasette",
]

@simonw
Copy link
Owner Author

simonw commented Aug 30, 2020

Annoyingly this seems to be the line that causes the circular import:

from .utils.asgi import Forbidden, NotFound, Response

@simonw
Copy link
Owner Author

simonw commented Aug 30, 2020

Weirdly even removing this single datasette import from utils/asgi.py didn't fix the circular import:

import json
from datasette.utils import MultiParams
from mimetypes import guess_type

@simonw simonw changed the title Simlify imports of common classes Simplify imports of common classes Sep 14, 2020
@simonw
Copy link
Owner Author

simonw commented Feb 6, 2022

I'm just going with:

from datasette import Response
from datasette import Forbidden
from datasette import NotFound
from datasette import hookimpl
from datasette import actor_matches_allow

@simonw simonw closed this as completed in 8a25ea9 Feb 6, 2022
@simonw
Copy link
Owner Author

simonw commented Feb 6, 2022

simonw added a commit that referenced this issue Mar 20, 2022
simonw added a commit that referenced this issue Mar 23, 2022
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

1 participant