convert promnesia to a namespace package #225

seanbreckenridge · 2021-04-12T23:56:42Z

initial attempt at converting this to a namespace package

this allows someone to install their own modules alongside promnesia.sources, and possibly override sources with their own changes

For reference: https://github.com/karlicoss/HPI/blob/master/doc/MODULE_DESIGN.org#namespace-packages

not being familiar with promnesia, was pretty painless to create a Source for my discord module

https://github.com/seanbreckenridge/promnesia
https://github.com/seanbreckenridge/HPI/blob/master/my/discord.py

seanbreckenridge · 2021-04-12T23:58:37Z

I expect some tests here to fail/the mypy commands in tox need to be changed

will probably just continue pushing changes here and you may squash this PR at the end

The only major concern is imports like

from promnesia import Source

like in the example config will no longer work, as I deleted the __init__.py file which was just redirecting imports to common

Theres no real way to add a deprecation warning/notice for that, as far as I know -- it'd be a breaking change

seanbreckenridge · 2021-04-13T00:18:12Z

ok -- even with the pyi files these are still considered namespace packages

In [1]: import promnesia

In [2]: promnesia.__path__
Out[2]: _NamespacePath(['/home/sean/Repos/promnesia-fork/src/promnesia', '/home/sean/Repos/promnesia/promnesia'])

In [4]: from promnesia.sources import discord

In [5]: discord.index
Out[5]: <function promnesia.sources.discord.index() -> Iterable[Union[promnesia.common.Visit, Exception]]>

seanbreckenridge · 2021-04-13T00:26:33Z

Hmm, this is a new one for me

https://github.com/karlicoss/promnesia/pull/225/checks?check_run_id=2328955562#step:5:723

mypy-core run-test: commands[1] | python -m mypy -p promnesia --exclude 'sources/*' --txt-report .coverage.mypy-core --html-report .coverage.mypy-core
Generated HTML report (via XSLT): /home/runner/work/promnesia/promnesia/.coverage.mypy-core/index.html
Generated TXT report (via XSLT): /home/runner/work/promnesia/promnesia/.coverage.mypy-core/index.txt
/home/runner/work/promnesia/promnesia/src/promnesia/misc: error: INTERNAL ERROR -- Please try using mypy master on Github:
mypy.rtfd.io/en/latest/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at python/mypy/issues
version: 0.812
/home/runner/work/promnesia/promnesia/src/promnesia/misc: : note: please use --show-traceback to print a traceback when reporting a bug
ERROR: InvocationError for command /home/runner/work/promnesia/promnesia/.tox/mypy-core/bin/python -m mypy -p promnesia --exclude 'sources/*' --txt-report .coverage.mypy-core --html-report .coverage.mypy-core (exited with code 2)

Seems to work fine locally:

$ python -m mypy -p promnesia --exclude 'sources/*'
Success: no issues found in 16 source files

Oh -- seems its related to generating the reports?

$ python -m mypy -p promnesia --exclude 'sources/*' --txt-report .coverage.mypy-core --html-report .coverage.mypy-core
/home/sean/Repos/promnesia-fork/src/promnesia/misc: error: INTERNAL ERROR -- Please try using mypy master on Github:
https://mypy.rtfd.io/en/latest/common_issues.html#using-a-development-mypy-build
If this issue continues with mypy master, please report a bug at https://github.com/python/mypy/issues
version: 0.812
/home/sean/Repos/promnesia-fork/src/promnesia/misc: : note: please use --show-traceback to print a traceback when reporting a bug
Generated HTML report (via XSLT): /home/sean/Repos/promnesia-fork/.coverage.mypy-core/index.html
Generated TXT report (via XSLT): /home/sean/Repos/promnesia-fork/.coverage.mypy-core/index.txt

Thought it might be related to the stub .pyi files? But after converting those back to .py files I get the same errors

seanbreckenridge · 2021-04-13T00:40:36Z

Seems it was trying to treat misc like a file instead of a directory for some reason?

https://mypy.rtfd.io/en/latest/common_issues.html#using-a-development-mypy-build
Please report a bug at https://github.com/python/mypy/issues
version: 0.812
Traceback (most recent call last):
  File "/usr/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/usr/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/usr/lib/python3.9/site-packages/mypy/__main__.py", line 23, in <module>
    console_entry()
  File "/usr/lib/python3.9/site-packages/mypy/__main__.py", line 11, in console_entry
    main(None, sys.stdout, sys.stderr)
  File "/usr/lib/python3.9/site-packages/mypy/main.py", line 90, in main
    res = build.build(sources, options, None, flush_errors, fscache, stdout, stderr)
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 179, in build
    result = _build(
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 253, in _build
    graph = dispatch(sources, manager, stdout)
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 2638, in dispatch
    process_graph(graph, manager)
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 2962, in process_graph
    process_stale_scc(graph, scc, manager)
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 3063, in process_stale_scc
    graph[id].finish_passes()
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 2175, in finish_passes
    free_tree(self.tree)
  File "/usr/lib/python3.9/contextlib.py", line 135, in __exit__
    self.gen.throw(type, value, traceback)
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 1941, in wrap_context
    yield
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 2170, in finish_passes
    manager.report_file(self.tree, self.type_map(), self.options)
  File "/usr/lib/python3.9/site-packages/mypy/build.py", line 804, in report_file
    self.reports.file(file, self.modules, type_map, options)
  File "/usr/lib/python3.9/site-packages/mypy/report.py", line 83, in file
    reporter.on_file(tree, modules, type_map, options)
  File "/usr/lib/python3.9/site-packages/mypy/report.py", line 482, in on_file
    for lineno, line_text in iterate_python_lines(path):
  File "/usr/lib/python3.9/site-packages/mypy/report.py", line 131, in iterate_python_lines
    with tokenize.open(path) as input_file:
  File "/usr/lib/python3.9/tokenize.py", line 392, in open
    buffer = _builtin_open(filename, 'rb')
IsADirectoryError: [Errno 21] Is a directory: 'src/promnesia/misc'

karlicoss · 2021-04-14T18:48:51Z

The only major concern is imports like
from promnesia import Source
like in the example config will no longer work, as I deleted the init.py file which was just redirecting imports to common
Theres no real way to add a deprecation warning/notice for that, as far as I know -- it'd be a breaking change

Yeah, I think it's not great -- it will break existing installs. We should probably keep it for now and add a deprecation warning -- it'll still allow having namespaced promnesia.sources package, right?

But good to switch tests and docs to not using it, I think we should keep just one test using from promnesia import ... for the same reason so we don't regress on it.

Seems it was trying to treat misc like a file instead of a directory for some reason?

Yeah, I think this is related to this mypy namespace package weirdness -- usually can be solved with a pyi file

seanbreckenridge · 2021-04-14T19:25:42Z

Seems that if the change is reverted, and promnesia/__init__.py exists, any subpackages can't be namespace packages.

> diff --git a/src/promnesia/__init__.py b/src/promnesia/__init__.py
new file mode 100644
index 0000000..cec15eb
--- /dev/null
+++ b/src/promnesia/__init__.py
@@ -0,0 +1,3 @@
+from pathlib import Path
+from .common import PathIsh, Visit, Source, last, Loc, Results, DbVisit, Context, Res
+
diff --git a/src/promnesia/__init__.pyi b/src/promnesia/__init__.pyi
deleted file mode 100644
index d436815..0000000
--- a/src/promnesia/__init__.pyi
+++ /dev/null
@@ -1,2 +0,0 @@
-# NOTE: without __init__.py/__init__.pyi, mypy behaves weird.
-# https://github.com/karlicoss/pymplate/blob/master/src/karlicoss_pymplate/__init__.pyi

[ ~ ] $ python3 -c "from promnesia.sources import reddit; from promnesia.sources import discord"
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ImportError: cannot import name 'discord' from 'promnesia.sources' (unknown location)

In order to use native namespace packages, like described in PEP420, you cant have an __init__.py in the top-level module. We'd have to switch to using pkgutil-styled/pkg-resource styled namespace packages, which are much less elegant and cause the barrier to entry to extend to go up.

So -- I don't see a way to convert this to a namespace without it being a breaking change -- can add the deprecation warning for now and then maybe in a few months/years actually convert it.

Hard to catch the warning as I'm not sure where its being triggered, but I modified the very basic test to use from promnesia import Source instead of from promnesia.common import Source

For now I can rename my package to install it as another module, and just import like from promnesia_sean.sources import ...

karlicoss · 2021-04-14T19:37:08Z

Yeah indeed, looked it up, and it seems that parent package prevents access to child namespace.
I was just about to cut off a release, will include your change as well!

And we can think about something in the meantime... maybe some mad dynamic magic to patch up the config?
Since the first entry point is still promnesia executable (don't think anything prevents namespaced package to have __main__?), and it simply loads the config file, then it could even be a matter of regex patch, or maybe something involving ast if we wanna be really fancy? So I feel like we're still in control here for the most part.

seanbreckenridge · 2021-04-14T19:53:05Z

Hmm -- I think the issue is still if promnesia/__init__.py exists, the promnesia.__path__ will never include more than one item -- so even if we knew there were other sources in __main__.py there'd be no way to discover them. Unless you just mean specifying the path to the repo in your config, and then loading it using importlib in __main__, but thats not much better than just installing it as a separate repo

If the purpose there is to allow overriding, its quite involved and I'd think thered only be 2 or 3 people using it before an eventual switch to namespace packages.

Currently I get get around 'overriding' by just renaming my sources.takeout to sources.google_takeout and importing from my promnesia_sean instead, I think thats fine for now - will bring it up again if theres some reason to try and do some magic in __main__.py

seanbreckenridge/dotfiles@6920802

seanbreckenridge/promnesia@cfaa905

karlicoss · 2021-04-14T19:59:26Z

Ah no -- I meant

remove promnesa/__init__py
add a patch here https://github.com/karlicoss/promnesia/blob/master/src/promnesia/config.py#L110 , in the simplest form something like p.read_text().replace(' promnesia ', ' promnesia.common ') (this is too broad, but you get the idea), and then save in some temporary location, and load the patched config from this tmp location.
So when the config is actually loaded, it won't do bare promnesia imports. Unless the user did something more elaborate, like splitting config into multiple parts, etc -- but hopefully in that case they would also easily fix the deprecation.

seanbreckenridge · 2021-04-14T20:00:47Z

Ah, patching the imports in the config -- yeah, I dont see an immediate problem with that

seanbreckenridge · 2022-01-27T15:12:29Z

Was looking into patching imports in the config. At least in 3.9, after ast.unparse was added - this looks to be pretty possible and robust -- would use a NodeVisitor/NodeTransformer to walk the ast, and then ast.unparse and dump the created python text in a temp directory, and import_config from that instead.

Doesn't seem as to do on 3.7 or 3.8 though, and would likely want to support that.

Currently I just think its more trouble than its worth, my modules are fine as a separate package and I just import stuff into my config file, all works fine

May look into traversing with ast just to find to see if there are any 'incorrect' imports directly from promnesia, and then try and do a regex match to fix them

convert promnesia to a namespace package

2dffeda

seanbreckenridge added 2 commits April 12, 2021 17:02

update 'from promnesia' imports

e457fc2

add pyi files for mypy

62b0806

add stub file for misc

fd7ee60

add deprecation warning for __init__.py

f2d5a11

karlicoss merged commit d211fb8 into karlicoss:master Apr 14, 2021

seanbreckenridge deleted the namespace-package branch October 27, 2021 23:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

convert promnesia to a namespace package #225

convert promnesia to a namespace package #225

seanbreckenridge commented Apr 12, 2021

seanbreckenridge commented Apr 12, 2021 •

edited

Loading

seanbreckenridge commented Apr 13, 2021

seanbreckenridge commented Apr 13, 2021 •

edited

Loading

seanbreckenridge commented Apr 13, 2021

karlicoss commented Apr 14, 2021

seanbreckenridge commented Apr 14, 2021 •

edited

Loading

karlicoss commented Apr 14, 2021

seanbreckenridge commented Apr 14, 2021

karlicoss commented Apr 14, 2021

seanbreckenridge commented Apr 14, 2021

seanbreckenridge commented Jan 27, 2022

convert promnesia to a namespace package #225

convert promnesia to a namespace package #225

Conversation

seanbreckenridge commented Apr 12, 2021

seanbreckenridge commented Apr 12, 2021 • edited Loading

seanbreckenridge commented Apr 13, 2021

seanbreckenridge commented Apr 13, 2021 • edited Loading

seanbreckenridge commented Apr 13, 2021

karlicoss commented Apr 14, 2021

seanbreckenridge commented Apr 14, 2021 • edited Loading

karlicoss commented Apr 14, 2021

seanbreckenridge commented Apr 14, 2021

karlicoss commented Apr 14, 2021

seanbreckenridge commented Apr 14, 2021

seanbreckenridge commented Jan 27, 2022

seanbreckenridge commented Apr 12, 2021 •

edited

Loading

seanbreckenridge commented Apr 13, 2021 •

edited

Loading

seanbreckenridge commented Apr 14, 2021 •

edited

Loading