Dhar / image-dreamer

"Dreams" images, such as shown in the Google Research blog post on "Inceptionism".
103 stars 42 forks source link

Cleanup; Add `main`. Change CLI parser to use Python stdlib's `argparse`. Configuration file. #9

Open tildebyte opened 9 years ago

tildebyte commented 9 years ago

I came up with this patch independently of @Tursaanpoika's nice work in #8. I think a proper main is always appropriate for a stand-alone command line script. @donaldm changes in #7 amount to a fork, IMHO. Also, I'm not sure why it's a good idea to use JSON (complex) over Python stdlib's ConfigParser (simple), or use the (almost) deprecated and more complicated optparse over argparse.

I'm looking next at pulling out the model vars (model_path, net_fn, param_fn) into a ConfigParser file, as I agree with @donaldm that it'd be nice for them to be easily configurable, and that it's a bit too involved for the command line. Also, it might be nice to option-ize the output image format and mention in the --input help some of the other formats which PIL can handle.

Tursaanpoika commented 9 years ago

I think @donaldm https://github.com/Dhar/image-dreamer/issues/7 had one more option in the code, scale. I'll grab your source and tinker around a bit :D argparse is what google came up when I searched on how to add cli -options to python, I'm no developer, but I still do like to nose around to see what I can do :)

Tursaanpoika commented 9 years ago

Oh, it was step scale, defaulting to 1.5

tildebyte commented 9 years ago

Right, I just spotted that myself and restructured things a bit.

donaldm commented 9 years ago

I opted to use JSON because I could represent the image mean list could be directly represented in the JSON instead of having to parse it out. Also I think JSON is a nice format which can support additions to the format.

I used OptionParser because I usually have to work with/support Python 2.4 (lol). I am just more familiar with it and I really don't see what argparse adds that I needed for this (but argparse works just as well :)).

I do agree my version is more of a fork though as I restructured things a good bit from the original code.

tildebyte commented 9 years ago

Awesome. Thanks for the comments. I have a ConfigParser commit almost ready. For this usage, it's really simple; it's also really easy to extend. I really like the readability of the cfg file itself.

I agree that parsing complex data could be a pain; I ran into the exact issue with mean which you mentioned ;)

donaldm commented 9 years ago

Are you using ConfigParser or SafeConfigParser?

Also I think the concept of mean needs to be looked at, because there are mean files potentially in the models themselves that are not being used currently. May need to evaluate this.

tildebyte commented 9 years ago

Safe.