search

Sei-Lisa / LSL-PyOptimizer

Optimizes a LSL2 script, folding constants, removing unused code and more. Adds new syntax features too.
GNU General Public License v3.0
35 stars 11 forks source link

Provide Python Packaging Information #15

Open nivardus opened 2 years ago

nivardus commented 2 years ago

Would it be possible to provide basic python packaging (setup.py/pyproject.toml, PEP-518) to simplify LSL-PyOptimizer installation? Were this done, the project could be installed via pip and the CLI automatically placed into the system path.

nivardus commented 2 years ago

Some changes to LSL-PyOptimizer would be required to make package installation possible. I've put together a cursory pass at making this possible using standard setuptools conventions: https://github.com/Sei-Lisa/LSL-PyOptimizer/compare/master...nivardus:pep-518 Does this seem like an acceptable route? If so, I would be willing to submit them as a PR. These changes can also be cribbed from as they are nothing special.

Thanks for the consideration.

Sei-Lisa commented 2 years ago

Thanks for your effort. I have to say, I am not a fan of PIP or setuptools, so please excuse me if I don't read the corresponding PEP.

Moving strutil to a folder makes sense, but I always had that feeling that it doesn't belong in lslopt/ because it's not related to the optimizer, as it's just a hack for Python 2/3 bilingualism (in fact it does more bilingual things than just strings, so the name could perhaps be changed). Maybe it could be on its own folder.

But I don't like the idea of moving main.py to lslopt/ at all. Current docs specify main.py as the executable, and the fact that it's on the top level helps users identify which file to run even without checking the docs. Furthermore, the name cli.py is not fully accurate, as it's used as a module by the test program. Besides, I don't see the reason to move the main program and not the test program.

Note that the preprocessor functionality is not enabled in this branch; the pcpp submodule is only there to facilitate rebasing of the preprocessor branch and switching between branches, and right now I don't recall what is cpreproc.py doing in this branch when it belongs to the other one. But including pcpp as the internal preprocessor in the main branch was not in my plans, for several reasons. One of them (security) is no longer a reason, but the others remain (namely, the fact that it's a dependency, the folder structure, and the recursive dependency on ply, all of which are related to your effort), and I don't want to offer this feature until I write my own preprocessor, which may well never happen, or find another simpler one which is easier to include.

All that said, if it's possible to add setup.py while respecting the above (ignoring preprocessor files, keeping main.py on the top level and moving strutil.py to its own folder, or not at all), I may consider it. In any case, installing via PIP should be an option, never a requisite.

nivardus commented 2 years ago

Hello, @Sei-Lisa. Thanks for the feedback and context about repository layout and content. It should be possible to meet all of the criteria you have outlined and still provide a setup.py file for those that want to install LSL-PyOptimizer as a package.

An executable main.py file can remain in the root of the directory provided it calls a main() method contained in a module which can also be targeted by setuptools. As you mention, "cli.py" may not be a precise name. The file containing this method could be called "cmd.py" or something else; I'm impartial and it is your project. Let me know if you have a preference. I've seen both cli.py and cmd.py used in popular projects.

I'll leave the processor files alone and can move strutil into its own module. Easy to do both.

The only major change needed to package LSL-PyOptimizer is that the data files builtins.txt and fndata.txt need to be moved into a module directory. This is required in order to place the files in an unambiguous python site-packages directory. These files could go into lslopt/data/, lslopt/ or similar. The builtins-unittest.txt test file can remain where it is.

I've staged changes in a new branch which can be compared at https://github.com/Sei-Lisa/LSL-PyOptimizer/compare/master...nivardus:package-less. If this seems acceptable I can open a PR for further review and possible incorporation.