Zenodo tests fail

[hseg]

The zenodo support added in #775 doesn't pass tests on my computer.

• papis version ($ papis --version or commit number): ee68780

Dependency versions on my system (Arch Linux)

python 3.11.8-1
python-arxiv 2.1.0-1
python-beautifulsoup4 4.12.3-1
python-bibtexparser 1.4.1-2
python-chardet 5.2.0-1
python-click 8.1.7-1
python-colorama 0.4.6-2
python-doi 0.2.0-1
python-dominate 2.9.1-1
python-filetype 1.2.0-2
python-habanero 1.2.3-2
python-isbnlib 3.10.14-2
python-lxml 4.9.2-3
python-prompt_toolkit 3.0.43-1
python-pyaml 23.9.0-2
python-pygments 2.17.2-1
python-pyparsing 3.1.2-1
python-requests 2.31.0-1
python-slugify 8.0.4-1
python-stevedore 5.2.0-1
python-tqdm 4.66.2-1
python-typing_extensions 4.10.0-1

how to reproduce the issue

From a clean checkout, run python -m pytest papis tests. Log attached.
errors.log

Judging by the output, it seems either the plugin or the golden output has stripped the HTML of the exported citation.

[jghauser]

could it be because of the new markdownify optional dependency?

[hseg]

That indeed fixed it. But semantically that's wrong -- if the tests depend on the presence of markdownify for correctness, it should at least be a test dependency

[jghauser]

Yeah that's right. Not sure whether it's better to change the tests such that they don't depend on markdownify or change the status of the dependency. As I wasn't involved in this, let's see what the others have to say :)

[alexfikl]

Good catch! Would a pytest.importorskip do the trick?

EDIT: We could change the tests too, but the CI already installs [develop,optional], which includes markdownify, so I think it's fine to skip it on local runs (?).

[hseg]

Checked, it works!
Also, I understand the markdownify optdepend is only relevant for zenodo imports for now?
Also, are any other tests conditional on the installed libraries? Presumably jinja and whoosh control some tests, but does anything else do so as well?

[alexfikl]

Checked, it works!

Thanks for checking!

Also, I understand the markdownify optdepend is only relevant for zenodo imports for now?

Yeah, it's just used to clean up some description / notes / abstracts from Zenodo. If it's not installed, it just leaves some HTML stuff in there, which is not the end of the world either!

Also, are any other tests conditional on the installed libraries? Presumably jinja and whoosh control some tests, but does anything else do so as well?

For libraries, yeah, it's just these 3 for now, which are also marked as optional dependencies in setup.py. Besides those,

• There are a few tests (e.g. in test_config) that only run on Linux because they use sed or something.
• There's also a few downloader tests that are disabled because we can't scrape them anymore, but haven't gotten around to deciding what to do with them.
• That should be it from a quick grep (?)

[hseg]

Ah sorry, marked success too soon -- the fix breaks the mypy tests

papis/zenodo.py:43: error: Unused "type: ignore" comment  [unused-ignore]
            from markdownify import markdownify  # type: ignore[import-untyped]
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
papis/zenodo.py:43:1: error: Cannot find implementation or library stub for module named "markdownify" 
[import-not-found]
            from markdownify import markdownify  # type: ignore[import-untyped]
    ^
papis/zenodo.py:43:1: note: Error code "import-not-found" not covered by "type: ignore" comment
papis/zenodo.py:43:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 2 errors in 1 file (checked 118 source files)

[alexfikl]

Ah sorry, marked success too soon -- the fix breaks the mypy tests

papis/zenodo.py:43: error: Unused "type: ignore" comment  [unused-ignore]
            from markdownify import markdownify  # type: ignore[import-untyped]
            ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
papis/zenodo.py:43:1: error: Cannot find implementation or library stub for module named "markdownify" 
[import-not-found]
            from markdownify import markdownify  # type: ignore[import-untyped]
    ^
papis/zenodo.py:43:1: note: Error code "import-not-found" not covered by "type: ignore" comment
papis/zenodo.py:43:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 2 errors in 1 file (checked 118 source files)

Hm, what version of mypy are you using? I tried with mypy 1.9.0 and I'm not getting any errors.

[hseg]

Using mypy 1.8.0

[hseg]

Checking with mypy 1.9.0, please wait

[alexfikl]

Checking with mypy 1.9.0, please wait

Reworked how markdownify gets ignored in #778 and that seems to pass on mypy 1.8.0 for me. Let me know if it works!

[hseg]

So testing with 1.10.0+dev.ea49e1fa488810997d192a36d85357dadb4a7f14 silences that error (and implies we probably should bump the minimal mypy version in deps), but raises another one:

papis/format.py:90:41: error: Argument 2 of "convert_field" is incompatible with supertype "Formatter";
supertype defines the argument type as "str | None"  [override]
        def convert_field(self, value: Any, conversion: str) -> Any:
                                            ^~~~~~~~~~~~~~~
papis/format.py:90:41: note: This violates the Liskov substitution principle
papis/format.py:90:41: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Found 1 error in 1 file (checked 118 source files)

Testing #778 now

[alexfikl]

papis/format.py:90:41: error: Argument 2 of "convert_field" is incompatible with supertype "Formatter";
supertype defines the argument type as "str | None"  [override]
        def convert_field(self, value: Any, conversion: str) -> Any:
                                            ^~~~~~~~~~~~~~~
papis/format.py:90:41: note: This violates the Liskov substitution principle
papis/format.py:90:41: note: See https://mypy.readthedocs.io/en/stable/common_issues.html#incompatible-overrides
Found 1 error in 1 file (checked 118 source files)

Hm, that seems to fail on 1.9.0, so maybe some stubs will get updated too with 1.10.0. In my experience, it doesn't seem possible to have things pass across version of mypy, so I'm generally happy if the CI passes :(

[hseg]

OK, so #778 indeed works with mypy 1.8.0 and markdownify, whoosh uninstalled. It also works with them installed.

[alexfikl]

Awesome! Thank you for helping out with all the testing!