dflook / python-minifier

Transform Python source code into its most compact representation
MIT License
553 stars 40 forks source link

Add module-level remove_unused_platforms option #97

Open garynthompson opened 7 months ago

garynthompson commented 7 months ago

Hi @dflook,

I am a hobbyist maker who has been using MicroPytohn on embedded devices with limited storage.

The current libraries being used have module-level if statements to create cross-platform compatible interfaces.

This feature was added in my fork so that when minifying a file, we can specify a target platform and all non-target platform blocks are excluded.

I modelled both the annotations and debug features and decided that this only will work at a module level

There is 100% code coverage on remove_unused_platform.py and I have confirmed that the Sphinx documentation compiles appropriately.

Please let me know if you are happy to include this upstream and if not, what changes are required.

Kind regards,

Gary

dflook commented 7 months ago

Hi @garynthompson, thanks for your effort on this PR. This is a good idea, and from a quick look the implementation looks good.

What libraries do you know of that use _PLATFORM in this way? It would be interesting to see how they determine it's value - I see you mentioned sys.uname or platform. A natural extension of this would be to automatically detect any such platform variable, but that would be a lot more effort.

Another thought I had is that this approach would work for any simple if condition, not just platform checks. I wonder if this could be generalised into a branch elimination transform that works with any simple condition. I don't know if it would have any use beyond platform checks though.

This looks good, i'll have a proper look soon.

garynthompson commented 7 months ago

Hi @dflook,

To partially generalise the solution, I also added some options where any basic keyword could be changed. Technically, any variable name can be passed in.

The current implementation can be used for other use cases, however, the command line arguments would be unintuitive as the naming of the main argument and options is around platform removal. The actual transform just needs a key (--platform-test-key=_PLATFORM) and a value (--platform-preserve-value=linux).

A natural extension of this would be to automatically detect any such platform variable, but that would be a lot more effort.

It is an interesting idea, I haven't explored the different ways people use to explore platform tests. I vaguely recall seeing platform tests much more often about 12 years ago, but I could be getting confused with C. I was curious, but not curious enough to spend a lot of time on it this weekend.

Originally I was just going to check simple if KEY == VALUE: statements, but then decided that elif and else are useful as well. I kept it to Ast.EQ for explicitness (I often keep PEP20 in mind).

Here is an example of a python module for controlling some hardware that is cross-platform that inspired me to look into this:

https://github.com/CoreElectronics/CE-PiicoDev-Unified/blob/main/PiicoDev_Unified.py

I am writing my own projects using the PiicoDev hardware using both Raspberry PI 4 and Raspberry Pi Pico.

Happy to make any improvements you suggest, even if it seems as seemingly minor as variable naming.