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:* >> >> >> <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 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/CAPGFgA2hpw1n%2B3JkynFc7ys6jzC_gV7MKn6g2AWQZDH8%3D7SLMA%40mail.gmail.com. For more options, visit https://groups.google.com/d/optout.
