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 support for the Logtalk language #1308

Closed
wants to merge 1 commit into from

Conversation

pmoura
Copy link
Contributor

@pmoura pmoura commented Jan 20, 2025

I noticed a single failed test, apparently due to use of a cell magic in the sample notebook. The cell magic is specific to the Logtalk kernel. Your help is most appreciated on how best to fix this test failure. Thanks.

========================================================================================================= FAILURES =========================================================================================================
_________________________________________________________________________________ test_ipynb_to_Rmd[ipynb_logtalk/logtalk_notebook.ipynb] __________________________________________________________________________________

ipynb_file = '/Users/pmoura/jupytext/tests/data/notebooks/inputs/ipynb_logtalk/logtalk_notebook.ipynb', no_jupytext_version_number = None

    def test_ipynb_to_Rmd(ipynb_file, no_jupytext_version_number):
>       assert_conversion_same_as_mirror(ipynb_file, "Rmd", "ipynb_to_Rmd")

/Users/pmoura/jupytext/tests/functional/round_trip/test_mirror.py:53: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
/Users/pmoura/jupytext/src/jupytext/compare.py:441: in assert_conversion_same_as_mirror
    compare(actual, expected)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

actual = "---\njupyter:\n  kernelspec:\n    display_name: Logtalk\n    language: logtalk\n    name: logtalk_kernel\n---\n\n# An...\n\n:- end_object.\n```\n\n## Sample query\n\n```{logtalk vscode={'languageId': 'logtalk'}}\nack::ack(2, 4, V).\n```\n"
expected = "---\njupyter:\n  kernelspec:\n    display_name: Logtalk\n    language: logtalk\n    name: logtalk_kernel\n---\n\n# An...\n\n:- end_object.\n```\n\n## Sample query\n\n```{logtalk vscode={'languageId': 'logtalk'}}\nack::ack(2, 4, V).\n```\n"
actual_name = 'actual', expected_name = 'expected', return_diff = False

    def compare(
        actual, expected, actual_name="actual", expected_name="expected", return_diff=False
    ):
        """Compare two strings, lists or dict-like objects"""
        if actual != expected:
            diff = difflib.unified_diff(
                _multilines(expected),
                _multilines(actual),
                expected_name,
                actual_name,
                lineterm="",
            )
            if expected_name == "" and actual_name == "":
                diff = list(diff)[2:]
            diff = "\n".join(diff)
            if return_diff:
                return diff
>           raise AssertionError("\n" + diff)
E           AssertionError: 
E           --- expected
E           +++ actual
E           @@ -9,7 +9,7 @@
E            # An implementation of the Ackermann function
E            
E            ```{logtalk vscode={'languageId': 'logtalk'}}
E           -%%load ack.lgt
E           +% %%load ack.lgt
E            
E            :- object(ack).

/Users/pmoura/jupytext/src/jupytext/compare.py:50: AssertionError
===================================================================================================== warnings summary =====================================================================================================
../../../usr/local/Caskroom/miniconda/base/envs/jupytext-dev/lib/python3.13/site-packages/sphinx_gallery/sphinx_compatibility.py:71
  /usr/local/Caskroom/miniconda/base/envs/jupytext-dev/lib/python3.13/site-packages/sphinx_gallery/sphinx_compatibility.py:71: RemovedInSphinx80Warning: The alias 'sphinx.util.status_iterator' is deprecated, use 'sphinx.util.display.status_iterator' instead. Check CHANGES for Sphinx API modifications.
    status_iterator = sphinx.util.status_iterator

tests/external/jupyter_fs/test_jupyter_fs.py::test_jupytext_jupyter_fs_metamanager
  /usr/local/Caskroom/miniconda/base/envs/jupytext-dev/lib/python3.13/site-packages/traitlets/traitlets.py:3615: DeprecationWarning: metadata {'per_key_traits': <traitlets.traitlets.Dict object at 0x11e3b60a0>} was set from the constructor. With traitlets 4.1, metadata should be set using the .tag() method, e.g., Int().tag(key1='value1', key2='value2')
    super().__init__(trait=trait, default_value=default_value, **kwargs)

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
================================================================================================= short test summary info ==================================================================================================
FAILED tests/functional/round_trip/test_mirror.py::test_ipynb_to_Rmd[ipynb_logtalk/logtalk_notebook.ipynb] - AssertionError: 
=========================================================================== 1 failed, 3129 passed, 161 skipped, 2 warnings in 187.05s (0:03:07) ============================================================================

Copy link

Thank you for making this pull request.

Did you know? You can try it on Binder: Binder:lab or Binder:notebook.

Also, the version of Jupytext developed in this PR can be installed with pip:

HATCH_BUILD_HOOKS_ENABLE=true pip install git+https://github.com/pmoura/jupytext.git@add_logtalk_language_support

(this requires nodejs, see more at Developing Jupytext)

@pmoura pmoura marked this pull request as draft January 21, 2025 10:32
@pmoura
Copy link
Contributor Author

pmoura commented Jan 21, 2025

Doing further testing, notice that conversions to hydrogen and rmd are incorrect when cell magic is present (e.g., %%load). Opening the file as a notebook strips the first % from cell magic, breaking it.

@pmoura pmoura force-pushed the add_logtalk_language_support branch from 63a2099 to 7a1208f Compare January 29, 2025 14:38
@mwouts
Copy link
Owner

mwouts commented Feb 4, 2025

Hello @pmoura , sorry for the long delay in responding. I expect to be more responsive from now on. Many thanks for your PR, that is very helpful! I will have a look to the failing test and comment back.

@pmoura
Copy link
Contributor Author

pmoura commented Feb 5, 2025

Btw, I converted the Markdown documentation of all Logtalk distribution examples (~200) so that Jupytext can open the files as notebooks. I have been testing primarily using VSCode with this extension. Happy with the results. Thanks for your work on Jupytext!

@pmoura pmoura force-pushed the add_logtalk_language_support branch from 7a1208f to 395a268 Compare February 6, 2025 21:14
@pmoura
Copy link
Contributor Author

pmoura commented Feb 6, 2025

Should I also update the CHANGELOG.md file?

@mwouts mwouts mentioned this pull request Feb 9, 2025
@mwouts
Copy link
Owner

mwouts commented Feb 9, 2025

Hey @pmoura , I've fixed the CI and will now give a look at the failing test.

What do you think of the second commit of #1310? Since the comment char is % in logtalk the magic commands are valid comments in logtalk script, aren't they?

@pmoura
Copy link
Contributor Author

pmoura commented Feb 9, 2025

Hey @pmoura , I've fixed the CI and will now give a look at the failing test.

What do you think of the second commit of #1310? Since the comment char is % in logtalk the magic commands are valid comments in logtalk script, aren't they?

Indeed Logtalk (and Prolog) use the % character for line comments. In the Jupyter kernel for Logtalk, all line and cell magic thus start with this character and thus are valid comments in Logtalk.

Thanks for fixing this issue in my pull request. Looking forward to the 1.16.7 release!

@pmoura pmoura marked this pull request as ready for review February 9, 2025 19:31
@mwouts
Copy link
Owner

mwouts commented Feb 10, 2025

Merged through #1310 - thank you @pmoura !

@mwouts mwouts closed this Feb 10, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants