Add KnotTheory as a provided package. #1358

rocky · 2021-05-06T16:58:49Z

No description provided.

mmatera

Just to know, is this loading now? Maybe we can add a few tests for this package.

rocky · 2021-05-06T19:51:31Z

Just to know, is this loading now? Maybe we can add a few tests for this package.

@mmatera Two tests have been added. But this now really slows down testing since loading KnotTheory is slow as is memory cleanup after the test finishes.

mmatera · 2021-05-06T20:07:56Z

OK, once we pass the tests, we can just comment it out, and keep it for when we have a faster kernel or a specific module.

rocky · 2021-05-06T20:09:52Z

OK, once we pass the tests, we can just comment it out, and keep it for when we have a faster kernel or a specific module.

Interestingly it works for both me and @soehms who I got it from. So there is something weird going on here.

Probably the test would just be that we can load the package.

But I need to defer looking at this until later.

mmatera · 2021-05-07T02:14:45Z

I checked it, and for some reason, the test passes if you run it alone
pytest test/test_knoththeory.py
, but fails if you run all the tests together with
pytest test
This does not changes if I change the name of the test to make that it run as the first test.

mmatera · 2021-05-07T02:19:29Z

Let's keep it commenting the test. We also need to put the reference of the original source of the knoththeory package in the AUTHORS.txt file.

rocky · 2021-05-07T04:06:59Z

I checked it, and for some reason, the test passes if you run it alone
pytest test/test_knoththeory.py
, but fails if you run all the tests together with
pytest test
This does not changes if I change the name of the test to make that it run as the first test.

interesting.

Let's keep it commenting the test. We also need to put the reference of the original source of the knoththeory package in the AUTHORS.txt file.

Sure. This is not the end of this.

rocky · 2021-05-07T08:35:55Z

@mmatera In 5626fbb we get closer.

If you know off hand the right rule for BeginPackage[string, needs_list] and have the time, please fill it in and uncomment the rule, and revert init.m to what it should be.

As for AUTHORS.txt it is not our practice to put here included Packages and that makes sense because that refers to people writing Mathics, not that packages it includes. And even of the libraries we use, we only mention that we use them.

Attribution would go inside the package and that makes sense too. When the package is moved the attribution goes with it. However, I don't see any special files giving credit or mention. We do refer to http://katlas.org/wiki/The_Mathematica_Package_KnotTheory but that doesn't give attribution either.

mmatera · 2021-05-07T14:13:43Z

test/test_knottheory.py

+
+
+@pytest.mark.skipif(not os.environ.get("KnotTheory", False),
+                    reason="set environment varable KnotTheory to run this test")


that's a clever solution!

mmatera · 2021-05-07T14:14:04Z

mathics/builtin/scoping.py

@@ -650,6 +649,9 @@ class BeginPackage(Builtin):
             Protect[System`Private`$ContextPathStack, System`$Packages];
             context
        """,
+        # "BeginPackage[context_String, needs_List]": """


Uncomment this

Given that when this is uncommented it causes a slew of errors in running the long-running test, I am not totally convinced it is correct. Possibly down the line there will be code checking that all items in the list are strings.

So for me at least this is a placeholder. I welcome though filling this out. We also probably need to add a test if it is in place.

In that case, let me suggest adding a rule like

"BeginPackage[context_String, needs_List]": """Print["Current kernel does not support the second argument ",needs, ". Please load them explicitly inside the package"]; BeginPackage[context]"""

in a way that the main package be loaded, and helps the user to detect what could go wrong.

mmatera · 2021-05-09T22:12:46Z

Regarding the full thing, since the tests are so heavy, I think that the best would be to maintain for now an independent repository for KnothTheory and Feyncalc mathics packages. We can test there against "experimental" specific branches inside Mathics, in a way that we can update on both sides as we make progress in the support for the required symbols.
At the same time, we can go adding new specific tests in the master branch of Mathics, inspired by those packages.

rocky · 2021-05-09T22:22:10Z

Regarding the full thing, since the tests are so heavy, I think that the best would be to maintain for now an independent repository for KnothTheory and Feyncalc mathics packages. We can test there against "experimental" specific branches inside Mathics, in a way that we can update on both sides as we make progress in the support for the required symbols.
At the same time, we can go adding new specific tests in the master branch of Mathics, inspired by those packages.

Sure, I tried setting this up separately, but wasn't able to do in the time I had allotted for it. If you want to do - go ahead.

For now this can just live here.

soehms · 2021-07-15T06:25:11Z

I've experimented with that branch (merged with current master) and it seems that there is a problem with importing dependent files.

If I work on master branch with a copy of the original files form http://katlas.org in the local path then the tests I made last May still work.

(mathics:30544): dbind-WARNING **: 23:23:14.323: Couldn't register with accessibility bus: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.

Mathics 3.1.1.dev0
on CPython 3.6.9 (default, Jan 26 2021, 15:33:00)
using SymPy 1.8, mpmath 1.2.1, numpy 1.19.5

Copyright (C) 2011-2021 The Mathics Team.
This program comes with ABSOLUTELY NO WARRANTY.
This is free software, and you are welcome to redistribute it
under certain conditions.
See the documentation for the full license.

Quit by evaluating Quit[] or by pressing CONTROL-D.

In[1]:= Needs["KnotTheory`"]
Loading KnotTheory` version of September 6, 2014, 13:37:37.2841.
        Read more at http://katlas.org/wiki/KnotTheory.
EndPackage::noctx: No previous context defined.
Syntax::sntxf: "\!\(\(PowerQ[_Integer] := True;\)" cannot be followed by "" (line 2729 of "./KnotTheory/init.m").
Out[1]= None

In[2]:= SymmetryType[Knot[5, 2]]
KnotTheory::credits: The symmetry type data known to KnotTheory` is taken from Charles Livingston's http://www.indiana.edu/~knotinfo/.
KnotTheory::loading: Loading precomputed data in IndianaData`.
Unset::norep: Assignment on SymmetryType for SymmetryType[KnotTheory`Indiana`K1_] not found.
Out[2]= Reversible

But, if I work on add-KnotTheoryPackage then I run into an infinite recursion:

(mathics:30457): dbind-WARNING **: 23:19:22.953: Couldn't register with accessibility bus: Did not receive a reply. Possible causes include: the remote application did not send a reply, the message bus security policy blocked the reply, the reply timeout expired, or the network connection was broken.

Mathics 3.1.1.dev0
on CPython 3.6.9 (default, Jan 26 2021, 15:33:00)
using SymPy 1.8, mpmath 1.2.1, numpy 1.19.5

Copyright (C) 2011-2021 The Mathics Team.
This program comes with ABSOLUTELY NO WARRANTY.
This is free software, and you are welcome to redistribute it
under certain conditions.
See the documentation for the full license.

Quit by evaluating Quit[] or by pressing CONTROL-D.

In[1]:= Needs["KnotTheory`"]
Loading KnotTheory` version of September 6, 2014, 13:37:37.2841 modified for Mathics.
        Read more at http://katlas.org/wiki/KnotTheory.
Warning: this package is mostly not working. It is provided as is in hope that people will help out to fix.
Out[1]= None

In[2]:= SymmetryType[Knot[5, 2]]
KnotTheory::credits: The symmetry type data known to KnotTheory` is taken from Charles Livingston's http://www.indiana.edu/~knotinfo/.
Mathics does not support the second argument: {KnotTheory`}. Please add Needs[] explicitly inside the package.
KnotTheory::loading: Message KnotTheory::loading not found.
Unset::norep: Assignment on SymmetryType for SymmetryType[KnotTheory`Indiana`K1_] not found.
KnotTheory::credits: The symmetry type data known to KnotTheory` is taken from Charles Livingston's http://www.indiana.edu/~knotinfo/.
Unset::norep: Assignment on SymmetryType for SymmetryType[KnotTheory`Indiana`K1_] not found.
KnotTheory::credits: The symmetry type data known to KnotTheory` is taken from Charles Livingston's http://www.indiana.edu/~knotinfo/.
..............

Observe, that you get Message KnotTheory::loading not found instead of Loading precomputed data in IndianaData, now.

Workaround: ``` BeginPackage["KnotTheory`", {"TubePlot`"}] ``` More generally, `BeginPackage[string, needs_list]` doesn't work. Run `test/test_knottheory.py` only if KnotTheory environment variable is set.

Update CHANGES.rst to reflect we have added KnotTheory.

And one that isn't without error.

rocky requested a review from mmatera May 6, 2021 16:58

mmatera reviewed May 6, 2021

View reviewed changes

rocky added the needs work label May 6, 2021

rocky marked this pull request as draft May 6, 2021 20:11

rocky force-pushed the add-KnotTheoryPackage branch from 6523d29 to 5626fbb Compare May 7, 2021 08:24

mmatera reviewed May 7, 2021

View reviewed changes

rocky force-pushed the add-KnotTheoryPackage branch from 70dc933 to cdeb740 Compare May 7, 2021 14:54

rocky force-pushed the add-KnotTheoryPackage branch from 1e94b57 to 4e8192f Compare May 12, 2021 09:26

rocky force-pushed the master branch 2 times, most recently from b007e8c to ef16eb6 Compare May 19, 2021 22:40

rocky force-pushed the master branch 5 times, most recently from 8367d69 to 83bb068 Compare June 7, 2021 21:01

rocky force-pushed the master branch 2 times, most recently from 9570fdd to 499f1bf Compare June 26, 2021 14:12

rocky added 2 commits July 15, 2021 05:19

Add KnotTheory as a provided package.

3bca4f8

Add tests for knottheory

fb727c1

rocky added 4 commits July 15, 2021 05:19

KnotTheory hacks and workarounds...

e66ec28

Workaround: ``` BeginPackage["KnotTheory`", {"TubePlot`"}] ``` More generally, `BeginPackage[string, needs_list]` doesn't work. Run `test/test_knottheory.py` only if KnotTheory environment variable is set.

Improve BeginPackage needs pattern...

82a7305

Update CHANGES.rst to reflect we have added KnotTheory.

Comment out really slow test

ae76ee0

And one that isn't without error.

Adjust 2-arg BeginPackage warning

8ea834f

rocky force-pushed the add-KnotTheoryPackage branch 2 times, most recently from ed53971 to 03cc2f0 Compare July 15, 2021 09:22

A few more tests

4dd291e

rocky force-pushed the add-KnotTheoryPackage branch from 4832059 to 4dd291e Compare July 15, 2021 09:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add KnotTheory as a provided package. #1358

Add KnotTheory as a provided package. #1358

rocky commented May 6, 2021

mmatera left a comment

rocky commented May 6, 2021 •

edited

mmatera commented May 6, 2021

rocky commented May 6, 2021

mmatera commented May 7, 2021

mmatera commented May 7, 2021

rocky commented May 7, 2021

rocky commented May 7, 2021

mmatera May 7, 2021

mmatera May 7, 2021

rocky May 7, 2021

mmatera May 7, 2021

mmatera commented May 9, 2021 •

edited

rocky commented May 9, 2021

soehms commented Jul 15, 2021



		@pytest.mark.skipif(not os.environ.get("KnotTheory", False),
		reason="set environment varable KnotTheory to run this test")

Add KnotTheory as a provided package. #1358

Are you sure you want to change the base?

Add KnotTheory as a provided package. #1358

Conversation

rocky commented May 6, 2021

mmatera left a comment

Choose a reason for hiding this comment

rocky commented May 6, 2021 • edited

mmatera commented May 6, 2021

rocky commented May 6, 2021

mmatera commented May 7, 2021

mmatera commented May 7, 2021

rocky commented May 7, 2021

rocky commented May 7, 2021

mmatera May 7, 2021

Choose a reason for hiding this comment

mmatera May 7, 2021

Choose a reason for hiding this comment

rocky May 7, 2021

Choose a reason for hiding this comment

mmatera May 7, 2021

Choose a reason for hiding this comment

mmatera commented May 9, 2021 • edited

rocky commented May 9, 2021

soehms commented Jul 15, 2021

rocky commented May 6, 2021 •

edited

mmatera commented May 9, 2021 •

edited