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

WIP Support for Celltotex #995

Closed
wants to merge 76 commits into from
Closed

WIP Support for Celltotex #995

wants to merge 76 commits into from

Conversation

mmatera
Copy link
Contributor

@mmatera mmatera commented Oct 31, 2020

In this branch, I am trying to include all the fixes needed to make that CellsToTeX package

Import@"https://raw.githubusercontent.com/jkuczm/MathematicaCellsToTeX/master/NoInstall.m"
This involves improvements in WL native Patterns support, Protect/Unprotect mechanisms, Names[], etc. The idea would be to merge this branch when the Import stops showing errors, including adding some tests.

@rocky
Copy link
Member

rocky commented Oct 31, 2020

I love this idea! I can find a number of other packages including some of the ones mentioned at sagemath that basically use the same kind of install mechanism.

I bet https://github.com/jkuczm/MathematicaSyntaxAnnotations#no-installation would be another one that would work because it outside of the install mechanism what's going on is pretty straightforward.

Maybe after this is done we can come up with a list of packages that can be installed this way and mention that in the repository somewhere.

@mmatera
Copy link
Contributor Author

mmatera commented Nov 5, 2020

Status update: several bugs and implementation issues on the Package and context subsystems were fixed.
The first part of CellsToTeX package loads without reporting problems:
BeginPackage@"CellsToTeX"
Import@"https://raw.githubusercontent.com/jkuczm/MathematicaSyntaxAnnotations/v0.2.2/SyntaxAnnotations/SyntaxAnnotations.m";
EndPackage[]
The second part Import@"https://raw.githubusercontent.com/jkuczm/MathematicaCellsToTeX/master/CellsToTeX/CellsToTeX.m"`
loads properly until the line 820, where starts raising messages connected with pattern matching functions.

@mmatera
Copy link
Contributor Author

mmatera commented Nov 6, 2020

A test that actually goes out to the internet, can be fragile: an internet connection might not be available and the URL can change, to name a couple of fragilities.

Marking the test with @pytest.skipif is one way to allow running the test conditionally. The flaky module is another way to indicate that a test might fail.

I added a line looking if the Import was successful. If it is, it performs the test. An improvement would be to raise a warning when this happens.

The problem with flaky and pytest.skipif is that I would need to know (ahead) if there is an internet connection, and the package is available.

@rocky , if you can give me a hand with the part of testing, I can continue with the fixings for the interpreter.

@rocky
Copy link
Member

rocky commented Nov 6, 2020

@rocky , if you can give me a hand with the part of testing, I can continue with the fixings for the interpreter.

Sure. Over the weekend I can add some commits in a branch off of this that you can look at.

@mmatera
Copy link
Contributor Author

mmatera commented Nov 7, 2020

Update: I found and fixed some issues with Systemp`Protect. Also, I included some support for DeleteCases with the levelspect parameter, and n. Still does not work, but now the number of errors is smaller...

@rocky
Copy link
Member

rocky commented Nov 7, 2020

Thanks for undertaking this.

Being able to do some sort of import of simple packages is very important. I am guessing that after this is done there will be many many more packages that we will be able to load which will add both power to Mathics and help draw in a community using and working on Mathics.

@rocky
Copy link
Member

rocky commented Dec 31, 2020

Just curious, How close is this to working? I also wonder what our chances are for importing other things of this kind are now?

@mmatera
Copy link
Contributor Author

mmatera commented Dec 31, 2020

Well, I am not sure when this package is going to work, but in the way I think we have improved a lot the support for the existing symbols. At this point, it seems that there are not fails in the import process, but for some reason, it still fails when the main routines are called. I hope to have more time during the next weeks to continue with this.

@rocky
Copy link
Member

rocky commented Jan 1, 2021

It might be good to break this PR up into little pieces that work and get that merged into master.

@mmatera
Copy link
Contributor Author

mmatera commented Jan 1, 2021

It might be good to break this PR up into little pieces that work and get that merged into master.

Actually, I did it and a couple of them were already merged. The remaining, working changes are in #1088

@@ -26,7 +28,11 @@ Enhancements and Bug fixes:

- Fix evaluation timeouts
- ``Sum``'s lower and upper bounds values can now be Mathics expressions

- Support for ``All`` as a ``Part`` espeficication
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmatera It would be good to merge this into master.

- Support for ``All`` as a ``Part`` espeficication
- Fix BeginPackage
- Improving support for OptionValue. Not it support list of Options.
- Adding support in ``from_python()`` to convert dictionaries in list of rules.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@mmatera It would also be good to get this into master

@@ -629,7 +631,7 @@ class BeginPackage(Builtin):
Append[System`Private`$ContextPathStack, $ContextPath];
$ContextPath = {context, "System`"};
$Packages = If[MemberQ[System`$Packages,$Context],
None,
$Packages,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think I've run into this problem a bit too, so it might be good to get this into master.

@mmatera
Copy link
Contributor Author

mmatera commented Jan 9, 2021

I open a new branch (PR #1102) with all these improvements. The only thing is that for some reason I can not do a rebase in order to keep just the changes. Maybe @rocky can help me to tide it up.

@rocky
Copy link
Member

rocky commented Jan 9, 2021

I open a new branch (PR #1102) with all these improvements. The only thing is that for some reason I can not do a rebase in order to keep just the changes. Maybe @rocky can help me to tide it up.

Sure. Will do. Oh - and by the way - thanks for all of the great work here!

@mmatera mmatera closed this Jan 9, 2021
@mmatera mmatera deleted the celltotexfixes branch January 9, 2021 23:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants