Fix doctests in dependencygraph.py

[ekaf]

Reduce the list of failing doctests (Issue #2989), by fixing the tests in nltk/parse/dependencygraph.py.

pytest --doctest-modules --doctest-continue-on-failure ./nltk/parse/dependencygraph.py

With this PR, if the "dot" binary is present, all tests pass. Otherwise, one test is skipped:

============================= test session starts ==============================
platform linux -- Python 3.9.2, pytest-7.1.2, pluggy-1.0.0
rootdir: [...]
plugins: forked-1.4.0, xdist-2.5.0, mock-3.8.2, cov-3.0.0
collected 3 items

nltk/parse/dependencygraph.py s.. [100%]

========================= 2 passed, 1 skipped in 2.58s =========================

[tomaarsen]

@Madnex perhaps this PR is useful for you, as it shows an example of how doctests can be conditionally skipped.
Looks good at a glance @ekaf. I'll try to get more time to actually have a good look soon.

[ekaf]

@Madnex @tomaarsen, rather than having several fixtures to check the availability of binaries, it seems more rational to have just one generic function that takes the binary name as parameter.

Then, the new test/binary_fixt can be imported in any test that requires the availability of a particular binary.

For ex. in the updated nltk/tag/hunpos.py:

Check whether the required "hunpos-tag" binary is available:

    >>> from nltk.test.binary_fixt import check_binary
    >>> check_binary('hunpos-tag')

Then:

pytest --doctest-modules --doctest-continue-on-failure ./nltk/tag/hunpos.py

============================= test session starts ==============================
platform linux -- Python 3.9.2, pytest-7.1.2, pluggy-1.0.0
rootdir: [...]
plugins: forked-1.4.0, xdist-2.5.0, mock-3.8.2, cov-3.0.0
collected 1 item

nltk/tag/hunpos.py s [100%]

============================== 1 skipped in 2.25s ==============================

[tomaarsen]

Seems like a good idea for sure

[ekaf]

Thanks for the review and suggestions @tomaarsen: I guess that LookupError is what you refer to when you call for something more specific.
I had not yet seen the f"string output, because pytest needs to increase verbosity twice for the message to be output without truncation:
pytest -vv --doctest-modules --doctest-continue-on-failure ./nltk/tag/hunpos.py

nltk/tag/hunpos.py::nltk.tag.hunpos.HunposTagger SKIPPED (Skipping test because the hunpos-tag binary was not found) [100%]

At this time, there don't seem to be other cases of unavailable binaries in #2989. There are a number unavailable "jar" packs related to the Stanford parser, but these will probably require a different solution.

However, the new check_binary() function can also replace the discourse, inference, and nonmonotonic fixtures in test/. I think it is preferable to do that within the present PR, in order to check that it works as intended.

[ekaf]

For ex. replace the redundant nltk/test/inference_fixt.py:

pytest -vv --doctest-modules --doctest-continue-on-failure ./nltk/test/inference.doctest

nltk/test/inference.doctest::inference.doctest SKIPPED (Skipping test because the mace4 binary was not found) [100%]

[tomaarsen]

Indeed, a LookupError seems best. This is looking good!

[stevenbird]

Looks like we're nearly ready to merge this... @ekaf are you intending to use check_binary() elsewhere in the test package?

[ekaf]

@stevenbird yes, this PR seems complete. I don't intend to use check_binary elsewhere.

[tomaarsen]

Thoughts on moving check_binary to https://github.com/nltk/nltk/blob/develop/nltk/internals.py, by the way?
It feels a bit odd to include "functionality" in the test folder, but that said, the functionality is exclusively for tests.

[ekaf]

Since it is just a test fixture among many others in the test directory, the current place seems adequate. However, maybe the filename could be changed to something broader (like for ex. setup_fixt.py), in order to open the possibility to include other generic fixtures later.
In particular, it could be relevant to also add a general function to replace the following:

grep importorskip nltk/test/*fixt.py

nltk/test/classify_fixt.py: pytest.importorskip("numpy")
nltk/test/gensim_fixt.py: pytest.importorskip("gensim")
nltk/test/probability_fixt.py: pytest.importorskip("numpy")

[ekaf]

Renamed fixture file with a broader filename, in order to accommodate the eventual addition of other generic fixtures.

The scope of this PR has already expanded beyond just handling dependencygraph.py, so I think it would be best to wait for a future PR to also replace pytest.importorskip(), because those cases are outside issue #2989.

[tomaarsen]

This is looking good. Thanks for this @ekaf!