Some minor feedback from me. 1. Use spaces, not tabs <https://www.python.org/dev/peps/pep-0008/#tabs-or-spaces>. 2. Use snake_case, not mixedCase <https://www.python.org/dev/peps/pep-0008/#descriptive-naming-styles> 3. More files, lose the # ------------- #
On the basis that one standard is better than many standards. Personally, it means I’ll more efficiently grok the contents of other peoples code without the distraction of personal style. In this case, I’m having trouble commenting on function and use because I can’t see past style. On 22 November 2015 at 13:06, Ben Hearn <[email protected]> wrote: > Hey Justin, > > I am working with the suggested changes and for the appdata dir path I > have this which works for me on Windows: > qAppData = > str(QtGui.QDesktopServices.storageLocation(QtGui.QDesktopServices.DataLocation)) > > Could you confirm this gives the correct OSX path for your local settings > dir? > > -- Ben > > On 21 November 2015 at 21:02, Justin Israel <[email protected]> > wrote: > >> >> On Sun, 22 Nov 2015 8:51 AM Ben Hearn <[email protected]> wrote: >> >> Hey Justin, >> >> Thanks for the feedback. I was hoping for some epic feedback so it's all >> good :) The more the better! >> >> The checking of the '/' was a windows thing I think, every time I would >> drop a directory in the directory path would start with a '/' so I decided >> to strip it. I was unsure as to whether this was a true issue. If this is >> not the case I will run some more tests and remove it if necessary. >> >> I will defo look into Qsettings. I know Qt has a whole bunch of handy >> modules like this but I have not explored them as of yet. >> >> As per showing the window in the class constructor would you recommend >> more along the lines of: >> >> if __name__ == "main": >> >> sys.app etc. etc. >> >> window = class() >> >> window.show() >> >> Ya this would be a better approach. Where the script decides to create >> and show the UI when it is run as a main executable, but leaves it free to >> be imported by others. >> >> >> Good catch on the event.accept() bug I will patch that up asap. >> >> Same thing with the recursive catch. Completely overlooked that I will >> patch that up asap also. >> >> I will document the use case for users functions, that is an excellent >> point to consider. >> >> Extending the program to handle bash and batch scripts is a cool >> extension as well I will definitely look into this. >> >> -- Ben >> >> >> On 21 November 2015 at 20:33, Justin Israel <[email protected]> >> wrote: >> >> Hey Ben, >> >> Thanks for sharing your work! >> >> I took a look at the script and gave it a quick test. Got some feedback, >> if you were interested... >> >> First problem for me was what you already expected. There are >> windows-specific concepts in the script, so it failed right away for me on >> OSX (and would fail on linux too). @line 34 >> <https://gist.github.com/ben-hearn-sb/dfd79ba2a7a77907cc77#file-file_runner-py-L34> >> you make an assumption that users will have "APPDATA" set in their >> environment, in order to join a directory path. I don't have that env key, >> so I immediately crashed with trying to join on a None type. I could >> quickly get past that by chucking in a default empty string value as the >> second parameter to getenv(). However, this results in every run of the >> tool creating a CUSTOM_DIR in my current working directory :-) A way to >> solve your intention is to take a look at using QSettings. This is a >> platform-independent way to save user preferences for your app. It will >> just go into the correct place for the correct platform. >> >> On line 40 >> <https://gist.github.com/ben-hearn-sb/dfd79ba2a7a77907cc77#file-file_runner-py-L40>, >> that "global dirIn" isn't needed. It's not doing anything here. >> >> L107 >> <https://gist.github.com/ben-hearn-sb/dfd79ba2a7a77907cc77#file-file_runner-py-L107>: >> Generally its not good practice to show UI windows from within a class >> constructor. It forces the caller to get a window that shows itself right >> away. Obviously it doesn't matter in the case where your script is only >> meant to execute, but if you ever wanted to offer your script as an >> importable module for someone to be able to integrate, and show this tool, >> it would make it less flexible. Similarly, at the end of the script, you >> directly call run(). If you put that under an "if __name__ == '__main__'" >> guard, then it would allow the script to be imported without running it. >> >> L126 >> <https://gist.github.com/ben-hearn-sb/dfd79ba2a7a77907cc77#file-file_runner-py-L126>: >> This is where I didn't get past, when I did a quick test of the tool. Here >> you are doing a check if the path starts with a "/", and stripping it off >> the path. So my absolute paths that were dropped were now being checked as >> relative paths to the current working directory, and were not found to >> exist. Is that the intent? Is there a very specific use case you expect >> people to do when running the tool and choosing with directories they are >> allowed to drop? Also, there is a small bug here in the logic (and >> similarly in scriptDropEvent), where if every path that is checked from the >> mime data does pass the check and the loop does a "continue", you end up >> getting to the "event.accept()" line anyways instead of "event.ignore()" >> >> L382 >> <https://gist.github.com/ben-hearn-sb/dfd79ba2a7a77907cc77#file-file_runner-py-L382>: >> I didn't really get to test this to confirm, but it seems like there is a >> bug here in how the loop breaks when it doesn't want to do a recursive >> operation on files. If the first directory you look at is empty, the walk >> will still continue to a subdirectory, which might have files, and in which >> case you then process once and then break to stop recursion. So you will >> get whatever subdirectory first has files, in the walk recursion. Seems >> like what you really want is to break no matter what, even if there are no >> files, to keep the walk from recursing into subdirs? >> >> While its great that you have nice module documentation at the top of the >> file, one thing it is missing is documenting the signature of the function >> you expect to find, when importing a users python script. I looked through >> the code and saw that you expect it to be "myFunction(filePath)" , but it >> would be good to add that to the documentation. Also, if you really want to >> make your application extensible, you could even support more than just a >> python script that has a specific function, by allowing any type of >> executable file that just expects 1 or more file paths to be passed as >> arguments. Someone could write a bash script, a bat file, python, ..., and >> you would run it in a subprocess. This would be slower to do on a per-file >> basis, so it would be something you might want to pass in batches of >> arguments. Anyways, just a thought and not something really needed. >> >> Hope this wasn't more feedback then you were looking for! >> >> Justin >> >> On Sun, Nov 22, 2015 at 7:05 AM Marcus Ottosson <[email protected]> >> wrote: >> >> Not a problem at all, I think it looks great! >> >> On 21 November 2015 at 17:52, Benjam901 <[email protected]> wrote: >> >> *Screen grab:* >> >> <img >> src="https://lh3.googleusercontent.com/-mYnbl7jHIQE/VlCuvlw7e_I/AAAAAAAACvk/lmcQ6M9JxPw/s1600/file_runner.jpg";> >> <https://lh3.googleusercontent.com/-mYnbl7jHIQE/VlCuvlw7e_I/AAAAAAAACvk/lmcQ6M9JxPw/s1600/file_runner.jpg> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> >> *gist readable format:* >> >> https://gist.github.com/ben-hearn-sb/dfd79ba2a7a77907cc77 >> >> -- Ben >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Python Programming for Autodesk Maya" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/python_inside_maya/f211a02d-bfff-42b8-979f-f1c9472f944c%40googlegroups.com >> <https://groups.google.com/d/msgid/python_inside_maya/f211a02d-bfff-42b8-979f-f1c9472f944c%40googlegroups.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> >> >> >> -- >> >> *Marcus Ottosson* >> [email protected] >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Python Programming for Autodesk Maya" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/python_inside_maya/CAFRtmOBk01VAU_V09KUCjDtZTnwyy3n2h%3DzugnhQO5iig__E0A%40mail.gmail.com >> <https://groups.google.com/d/msgid/python_inside_maya/CAFRtmOBk01VAU_V09KUCjDtZTnwyy3n2h%3DzugnhQO5iig__E0A%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> For more options, visit https://groups.google.com/d/optout. >> >> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "Python Programming for Autodesk Maya" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/python_inside_maya/kAYHaKvnJEY/unsubscribe >> . >> To unsubscribe from this group and all its topics, send an email to >> [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/python_inside_maya/CAPGFgA2hpw1n%2B3JkynFc7ys6jzC_gV7MKn6g2AWQZDH8%3D7SLMA%40mail.gmail.com >> <https://groups.google.com/d/msgid/python_inside_maya/CAPGFgA2hpw1n%2B3JkynFc7ys6jzC_gV7MKn6g2AWQZDH8%3D7SLMA%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> >> >> For more options, visit https://groups.google.com/d/optout. >> >> >> >> -- >> >> <img src=" >> https://ci5.googleusercontent.com/proxy/lLPG1JnWaCc_GbeATW0_5G1SlKYPuumIQE1Uu7_6KGKh-Hz5k0w0cLhaLRwTEjK9OGpJo2SgzzKJR_y7hC9pbg03YSw0u1oX8YcsQ8Wc55EqqJM_0u1-j-zV44WxVeUl=s0-d-e1-ft#https://docs.google.com/uc?id=0Bz5FnMS-dH_kWTdLeGFVYXUySW8&export=download >> "> >> >> Tel - +46 76245 92 90 (Sweden) >> >> LinkedIn: http://www.linkedin.com/pub/ben-hearn/50/a64/33b >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Python Programming for Autodesk Maya" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/python_inside_maya/CAM2ybkUjJaztV40Uxs_xP3SBK%2BjHfafoUusgy7VLJgV2gh70Mg%40mail.gmail.com >> <https://groups.google.com/d/msgid/python_inside_maya/CAM2ybkUjJaztV40Uxs_xP3SBK%2BjHfafoUusgy7VLJgV2gh70Mg%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> For more options, visit https://groups.google.com/d/optout. >> >> >> -- >> You received this message because you are subscribed to a topic in the >> Google Groups "Python Programming for Autodesk Maya" group. >> To unsubscribe from this topic, visit >> https://groups.google.com/d/topic/python_inside_maya/kAYHaKvnJEY/unsubscribe >> . >> To unsubscribe from this group and all its topics, send an email to >> [email protected]. >> To view this discussion on the web visit >> https://groups.google.com/d/msgid/python_inside_maya/CAPGFgA00aMAEz19Ctikf%2BqA79%3DnX_SsdswxTQMFj4FguRsOaWQ%40mail.gmail.com >> <https://groups.google.com/d/msgid/python_inside_maya/CAPGFgA00aMAEz19Ctikf%2BqA79%3DnX_SsdswxTQMFj4FguRsOaWQ%40mail.gmail.com?utm_medium=email&utm_source=footer> >> . >> >> For more options, visit https://groups.google.com/d/optout. >> > > > > -- > > Tel - +46 76245 92 90 (Sweden) > LinkedIn: http://www.linkedin.com/pub/ben-hearn/50/a64/33b > > -- > You received this message because you are subscribed to the Google Groups > "Python Programming for Autodesk Maya" group. > To unsubscribe from this group and stop receiving emails from it, send an > email to [email protected]. > To view this discussion on the web visit > https://groups.google.com/d/msgid/python_inside_maya/CAM2ybkVsYnZ%3Dogam765fWrUTinkpWjs_3O32FM-tVRAXsFtGFg%40mail.gmail.com > <https://groups.google.com/d/msgid/python_inside_maya/CAM2ybkVsYnZ%3Dogam765fWrUTinkpWjs_3O32FM-tVRAXsFtGFg%40mail.gmail.com?utm_medium=email&utm_source=footer> > . > > For more options, visit https://groups.google.com/d/optout. > -- *Marcus Ottosson* [email protected] -- You received this message because you are subscribed to the Google Groups "Python Programming for Autodesk Maya" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To view this discussion on the web visit https://groups.google.com/d/msgid/python_inside_maya/CAFRtmOAf_L6%3Dys7zXZ8BNwZ1CsB3j1MLrQ1dJj5uupcOeQ4-7A%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
![https://lh3.googleusercontent.com/-mYnbl7jHIQE/VlCuvlw7e_I/AAAAAAAACvk/lmcQ6M9JxPw/s1600/file_runner.jpg"](https://lh3.googleusercontent.com/-mYnbl7jHIQE/VlCuvlw7e_I/AAAAAAAACvk/lmcQ6M9JxPw/s1600/file_runner.jpg"){kind=link}
