Skip to content

Conversation

@rocky
Copy link
Member

@rocky rocky commented Nov 2, 2020

These are the changes need to make PyMathics fully self sufficient.

There might be more to do in test.py to modularize so that it can be used in PyMathics modules to run their tests, but that will come later.

Diitto for the issue of collecting PyMathics documentation into a single PDF.

I've tried this both on the independent PyMathics modules hello and natlang.

Note: some files have been reformated using black which accounts for the large changes in tnose files.

rocky added 4 commits October 31, 2020 22:19
Works for both pymathics-hello and pymathics-natlang

However pattern parsing is a little funky and should be improved.
this time for sure!
Allow PyMathics functions and variabls to be called without qualifying
the PytMathics context.

With this, I thin last bit of work in to be done mathics is complete.
@rocky rocky requested review from GarkGarcia and mmatera November 2, 2020 02:16
@mmatera
Copy link
Contributor

mmatera commented Nov 2, 2020

Thanks for this, it looks very nice! My only doubt is regarding the context in which symbols in pymodules should be loaded. But as it is it should work.

# at the beginning.
context_path = evaluation.definitions.get_context_path()
if "PyMathics`" not in context_path:
context_path.insert(0, "PyMathics`")
Copy link
Contributor

Choose a reason for hiding this comment

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

The context of the symbols in the pymathics module should be pymathics or pymathicsmodulenale` ?

Copy link
Member Author

@rocky rocky Nov 3, 2020

Choose a reason for hiding this comment

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

This is an interesting question for discussion.

A short summary of my thinking right now is PyMathics for now and maybe later PyMathics<tick>module later. But please give me your thoughts.

Let me review history.

In v1.0 natlang was a "buitin" and so its context was I guess System. Do I have that right? I get conflicting reports from mathicsscript --full-form:

In[1]:= Simpify[x 2 - x]
Global`Simpify[System`Plus[System`Times[Global`x, 2], System`Times[-1, Global`x]]]
Out[1]= Simpify[x]
In[2]:= ?Simplify
System`Information[System`Simplify, System`Rule[System`LongForm, System`False]]

I am guessing that first Global is wrong or incomplete

Note that right now, we don't break out "System" functions and variables, although they in fact are in different Python modules/files.

So then we split this off. As best as I can tell, dealing with the context might have gotten messed up or had been incomplete. At least, I had a hard time trying get it to the consistentcy it is now.

The choice here was to give all PyMathics modules a PyMathics context rather than add this to System which I think happened in some (all?) cases.

And in doing that, part of the problem I ran into was with in parser.convert.py which tries to add the context prefix and seems to get it wrong occasionally as seen above with Global

The downside of putting a lot of stuff under a single context is the possibility for name clashes. And that is of course fixed by adding the module name. I guess we could get the name from pymathics_version_data.name.

But in practice, right now there are going to be very few PyMathics modules (that is until Gark writes the definitive guide for writing Mathics builtins).

Probably the only PyMathics modules that are going to exist are the ones we create, which might number as many as 4. There is NatLang and in the future Graph. Possibly the Chemical Data should be pulled out too since that feels a bit specialized, like Natural Language Processing.

(Note: At some point I'll write a pytest for LoadModule which will basically create the Hello PyMathics module inside the test since that code is so short).

So while I think ultimately this is a good thing to do, right now this PR is a bit large.

And there are more things to do just related to this. Renaming test.py to doctest.py and making sure it can be used as a module for testing from PyMathics is an example.

So in sum, my opinion is let's just do this step, let it settle and work out the kinks. And then continue doing more ambitious, more correct, other kinds of things.

Copy link
Contributor

Choose a reason for hiding this comment

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

As I told you before, I think this PR is a great improvement, and I would merge it as it is. What I wanted to point out that the scope of the modules is something to think about it in the future, and I wanted to have your impression of what would be the best way to handle it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ok -- thanks. We are in agreement then.

As you say, it is something to do for the future.

Feel free to open issues if that's the way you want to go. For myself, there are so many things that I am aware of that should be addressed, issues in general haven't been that helpful to me as it seems for others.

@rocky rocky merged commit c280808 into master Nov 3, 2020
@rocky rocky deleted the pymathics-fixup branch November 8, 2020 02:54
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.

3 participants