search

muodov / kociemba

A pure Python and pure C ports of Kociemba's algorithm for solving Rubik's cube
GNU General Public License v2.0
561 stars 126 forks source link

Added solve.py and verify.py, python3 importing, C 'make clean' and 'make install' #25

Closed dwalton76 closed 7 years ago

dwalton76 commented 7 years ago

This change is Reviewable

muodov commented 7 years ago

@dwalton76 thanks for your PR! I'm really happy seeing someone interested in this project and I definitely appreciate your effort. Sorry for delaying this, I was a bit busy with other work and this PR didn't seem very urgent.

That said, I have some remarks about your changes:

  1. Although the C implementation can be used on its own, the main goal of this project is a Python package. I think it makes sense to keep this as a main way of using it, because it allows using python tooling for installation and deployment.
  2. As a Python package, kociemba provides a universal installation script (setup.py) and it does generate a command-line executable entry point (see readme). So I believe make install and solve.py are not really needed. One should use python setup.py install (this is done automatically if you use pip) and a kociemba command.
  3. verify.py command might be useful, but I think it should be implemented the same way as the kociemba command: using setup.py's entry_point and a function in command_line.py
  4. I don't quite get why the imports change? They are tested under Python 3, and it works fine.

So if you address the points above, I will be happy to merge your PR

dwalton76 commented 7 years ago

@muodov My apologies for being slow to reply, I forgot all about this pull request :( I remembered it when I did the "Disable parity checks produced in 4x4x4 reduction" this week, I'm sure that is something you don't want to bring in. I'll re-issue the pull request once we figure out what parts should/shouldn't go in.