py-earth icon indicating copy to clipboard operation
py-earth copied to clipboard

Remove *.c files in repository

Open aleon1138 opened this issue 10 years ago • 3 comments

I propose we modify setup.py to always cythonize by default and remove the *.c files from the repository. It does not take any time to regenerate them and they complicate commits and diffs. Also I would assume that to build on different platforms the *.c files would be different anyways.

aleon1138 avatar Apr 15 '15 18:04 aleon1138

Right, I keep them in for two reasons:

  1. Consistency with scikit-learn, which keeps them in.
  2. Elimination of cython dependency/sensitivity to cython version

I do not believe the C files are platform dependent. However, I agree that they complicate commits and diffs. In fact, I often mess up and forget to commit the changed C files. I've been maintaing this project on my own for a long time, so it hasn't been a problem. Now that other people have started participating (which I am very happy about:)) it is more of a big deal to do things cleanly. I think if we put py-earth up on pypi, then the C files can be included there. That would mean users who just want an easy way to install would have it and contributors wouldn't have to deal with C files all the time (just for pypi builds). If we then want to merge into sklearn, it would be easy enough to add them back, so reason 1 is not a huge concern. So I guess I am in favor of this provided we do a release on pypi that includes the C files. However, I have never done such a thing. I can look into it, but it might take me some time to get to it (I'm traveling for the next month). If you can figure out the steps and what you need from me, I can probably get it done much faster.

jcrudy avatar Apr 18 '15 08:04 jcrudy

@jcrudy " I think if we put py-earth up on pypi, then the C files can be included there", By the way, can we add pyearth on pypi?

mehdidc avatar Apr 27 '15 16:04 mehdidc

Consistency with scikit-learn, which keeps them in

scikit-learn removed the C files from the repository recently (but it includes them in the release tarballs).

mblondel avatar Mar 28 '16 02:03 mblondel