-
Notifications
You must be signed in to change notification settings - Fork 163
Make installable with setup.py #33
base: master
Are you sure you want to change the base?
Make installable with setup.py #33
Conversation
|
Whoops. I was wrong about the dependency issues. This package depends upon |
@augustt198, really love the work! @talwrii This is a great start towards packaging! I'll sprinkle some comments throughout. Been meaning to post here for a while! I don't know much about antlr, latex, or really sympy for that matter, but I've worked with folks that would really like to have tools like this in their work and teaching toolbelt. I built up the various packages in Here are the recipes: https://github.com/bollwyvl/staged-recipes/tree/latex2sympy/recipes You can get them all built with: conda install conda-build
conda build -c conda-forge recipes/latex2sympy # should build its deps! The |
def compile_grammar(): | ||
here = os.path.dirname(__file__) or '.' | ||
package_dir = os.path.join(here, 'latex2sympy') | ||
subprocess.check_output(['antlr4', 'PS.g4', '-o', 'gen'], cwd=package_dir) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure what a "normal" install of this would be like, but I ended up needing an antlr.cmd
wrapper, so I had to do some platform checking.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's good having a Windows user to test this, you have my sympathy :). Seems like a good change.
How hard is it to install antlr on windows? I don't really understand the windows development environment. If antlr is hard to install and not installed by default by the kind of people who would use this package then requiring people to install antlr could prevent adoption.
On debian antlr looks like this: https://packages.debian.org/stretch/all/antlr4/filelist
Conda on linux calls the executable antlr
(rather than antlr4
)
https://anaconda.org/conda-forge/antlr/files
$ tar --list -f ~/Downloads/antlr-2.7.7-5.tar.bz2 | grep bin
bin/antlr-config
bin/antlr
I'm not sure whether the windows conda antlr package even works :/
How is antlr installed on your machine?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How hard is it to install antlr on windows? How is antlr installed on your machine?
Well, I built antlr4
as a conda package on osx/linux/win... and by "built", i mean dumped the .jar
in place and made a wrapper script, rather than diving into maven
witchcraft. Usually, this is acceptable for java-based commands... can't speak to anyone using conda
as a java package manager. I'm not really a windows user (just used a VM), but The Conda Way is to, whenever possible, take care of that community, as people do Get Work Done on windows.
Conda on linux calls the executable
The linked version on conda-forge (2.7.7) is likely different enough to require a new package anyway. I didn't even test it, since the docs said antlr4
.
requiring people to install antlr could prevent adoption.
Anyhoo... in testing, once the the packages are built, they just install, no problem.
But the bigger story is that antlr4
isn't even required at runtime of latex2sympy
, just the runtimes (also built as conda packages). This means it doesn't strictly even need to be in conda
, and indeed, include_package_data
works with wheels as well (though I haven't tested that route). Instead of checking the gen
folder in, just distributing it in your package is just fine, shifting the burden to release time rather than every-user-install time, which is usually a really good trade.
setup.py
Outdated
author='august.codes', | ||
author_email='augustt198@gmail.com', | ||
description='Parse latex markup into sympy: suitable for programmatic modifcation', | ||
license='GPLv3', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Whoops! Looks like it's MIT below, which should have been changed here!
setup.py
Outdated
author_email='augustt198@gmail.com', | ||
description='Parse latex markup into sympy: suitable for programmatic modifcation', | ||
license='GPLv3', | ||
keywords='MIT', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could definitely use some more stuff, I suppose...
setup.py
Outdated
description='Parse latex markup into sympy: suitable for programmatic modifcation', | ||
license='GPLv3', | ||
keywords='MIT', | ||
url='', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems like the main repo URL would be reasonable.
setup.py
Outdated
license='GPLv3', | ||
keywords='MIT', | ||
url='', | ||
packages=['latex2sympy'], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i just like to use setuptools.find_packages
cmdclass=dict( | ||
install=AntlrInstallCommand, | ||
develop=AntlrDevelopCommand), | ||
test_suite='nose.collector' |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could just go to pytest...
setup.py
Outdated
|
||
setuptools.setup( | ||
name='latex2sympy', | ||
version=0.1, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be a string, and should ideally appear in exactly one place (i.e. latex2sympy/_version.py
) and be read here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it already appears in one place: setup.py
. But adding a _version.py
(or perhaps __version__
in the package __init__.py
and pulling this in would make a version number accessible from code. I can see the value of this when trying to work out exactly what version of this package is running. On the other hand, there is a price in terms of discoverability and I don't imagine this package particularly changing a great deal.
install=AntlrInstallCommand, | ||
develop=AntlrDevelopCommand), | ||
test_suite='nose.collector' | ||
) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In addition, I added include_package_data
as well as a MANIFEST.in
to capture all the extra files.
a7e20d3
to
f2fbb52
Compare
import * was causing nose to run sympy tests
This includes magic to generate antlr files when installed. This works with `setup.py develop`, `setup.py install` and `pip`. It does not, however, work with tox. (http://stackoverflow.com/questions/43897581/python-packaging-generate-a-python-file-at-installation-time-have-this-work-wi)
In light of simpler installation mechanism and new package structure. Include instructions on building grammar in develop instructions.
We do not want to depend upon antlr-ast rather the verbosely named antlr4-python3-runtime. This has different versions depending upon which version of python we are using.
f2fbb52
to
f3d3dc6
Compare
|
||
setuptools.setup( | ||
name='latex2sympy', | ||
version="0.1.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ooops, sorry about the quotes!
I made this binder so you can see how stuff is installed: https://mybinder.org/v2/gh/bollwyvl/latex2sympy-demo/master
|
Hi guys! What do you think about just committing the parser into the git tree? It seems to be a lot simpler than Including I am interested in using |
To quote myself in the initial comment:
Yes, I think no one commenting on this issue considers themselves to be "responsible" for the project. I think the initial developer might be busy with other things. Given this, I think a friendly fork with a name change might be in order. I would suggest:
Maintainers is a big word. Basically we just need someone to accept pull requests. My understanding is/was that the original author is a high school student... so I imagine he might have moved on to new things. No judgement here! I just think high school students might be inclined to change their interests more rapidly than those whose interests are influenced by their income! An indication to the contrary on the behalf of the original author is more than welcome. |
Hey folks... As tacitly approved over in #1, we've started bringing this
codebase into sympy proper:
sympy/sympy#13706
Please chime in there if you have some more feedback!
…On Fri, Dec 29, 2017, 16:53 talwrii ***@***.***> wrote:
What do you think about just committing the parser into the git tree?
To quote myself in the initial comment:
An alternative approach is to commit the generated grammar into the source
tree,
possibly adding a test to ensure that it hasn't been updated. This has the
advantage of > removing the dependency upon ANTLR for installation and
converting evil magic into > tests. This is likely there approach I would
use if this were my own project. How happy > you are with this depends upon
your DRY sensibilities.
Yes, antlr seems like such a pain to install on some systems
(particularly windows) that this would appear to be a good idea. On the the
other hand, people can get cranky if you try to introduce redundancy into
their pet projects!
I think no one commenting on this issue considers themselves to be
"responsible" for the project. I think the initial developer might be busy
with other things. Given this, I think a friendly fork with a name change
might be in order.
I would suggest:
- Emailing / dming / redditing the author asking for permission (Yes
this is licensed under MIT, but there is such a thing as politeness)
- Forking with a new name and committing the anltr files
- Find a group of data scientists / scientists to be "maintainers".
This probably isn't me as I don't use this tool on a regular basis.
- Release (publicise)
Maintainers is a big word. Basically we just need someone to accept pull
requests.
I feel as if a new name is necessary to indicate new maintainership,
though one imagines if the author could be convinced to link to this new
project that might work as well.
My understanding is that the original author high school student... so I
imagine he might have moved on to new things. No judgement here! I just
think high school students might be inclined to change their interests more
rapidly than those whose interests are influenced by their income!
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACxRFXYTBQIiVJtF9GRWdk8V73CA_WNks5tFV9CgaJpZM4NW7fs>
.
|
Good idea: make sympy responsible for accepting pull requests :) |
That, and we should be able to make use of more of the machinery for
testing, etc. It will be marked experimental, as we did evaluate using some
other technologies like ply, but at least for a while this will be the
baseline.
At present, we've handled all of the dependency stuff, and are fighting
through all the licensing issues.
Once it's in, we'll want to visit the other forks, and leave some notes to
bring other changes in. On the whole, it should be great!
…On Sat, Dec 30, 2017 at 3:43 PM talwrii ***@***.***> wrote:
codebase into sympy proper:
Good idea: make sympy responsible for accepting pull requests :)
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#33 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AACxRMP1cs5Q_9whPeOgWUy1KXEqGKpmks5tFqBsgaJpZM4NW7fs>
.
|
There are various advantages to a project being setuptools-compatible:
python setup.py install git+https://github.com/augustt198/latex2sympy#egg=latex2sympy
(no additional steps). This encourages adoptionI have made this library python3 compatible.
This is because ((Edit: we only depend uponanltr
in their wisdom) have made the newest version of their library incompatible with python2.antlr4-python[23]-runtime
notantlr-ast
)Caveats
Evil magic to build code at install time
(Edit: This library uses a generated parser that is stored in
latex2sympy/gen/
).There's a bit of evil magic here. Python's support for code generation during a build step leave a little to be desired. I would class the approach use as a hack.
An alternative approach is to commit the generated grammar into the source tree, possibly adding a test to ensure that it hasn't been updated. This has the advantage of removing the dependency upon ANTLR for installation and converting evil magic into tests. This is likely there approach I would use if this were my own project. How happy you are with this depends upon your DRY sensibilities.
This changes the package structure.
This changes the package structure. This could break pre-existing code. We could work around this by installing individual modules rather than a package. I think there's a strong argument for not having a module in the global namespace called
process_latex
. How happy you are with this approach depends upon how happy you .are to inconvenience existing users.