Update liberbatch.py #9
Conversation
|
@CThierrin You may already know this, but in the upper-right corner you can request reviews from people. I will generally wait to review until requested so that I know that you're ready for it and so I don't step on your toes :) |
There was a problem hiding this comment.
Left a bunch of comments...some are out-of-scope of the issues referenced, so feel free to address them here or make new issue(s).
This PR could also use a more descriptive name (what are you updating?). And for future changes, your commit messages need more detail (again, what are you updating? If I needed to go back later and find the specific commit where something was changed, how would I do that?)
liberbatch.py
Outdated
| for mefile in os.listdir(directory): | ||
| filename = os.fsdecode(mefile) | ||
| if filename.endswith("corr.mei"): | ||
| liberupdate.main(filename) | ||
| liberupdatev5.main(filename) | ||
| print(filename +" has been updated") | ||
| err= err+ (meichecker.main(filename[:-4]+"NEW2.mei")+"\n") | ||
| # print(err) |
liberbatch.py
Outdated
| @@ -1,19 +1,15 @@ | |||
| import os, meichecker, liberupdatev5 | |||
There was a problem hiding this comment.
Are you using a code linter? This is not PEP8 compliant (https://peps.python.org/pep-0008/#imports)
| @@ -1,19 +1,15 @@ | |||
| import os, meichecker, liberupdatev5 | |||
| PATH = "your path here" | |||
There was a problem hiding this comment.
If you want the user to go in and change the path, that should be documented. I think a command line argument would be generally preferable.
liberbatch.py
Outdated
| err="" | ||
|
|
||
| i = 0 #for debugging | ||
| for mefile in os.listdir(directory): | ||
| filename = os.fsdecode(mefile) | ||
| if filename.endswith("corr.mei"): |
There was a problem hiding this comment.
This looks like a requirement (that the files end with corr.mei)! This should be documented somwhere.
liberbatch.py
Outdated
| for mefile in os.listdir(directory): | ||
| filename = os.fsdecode(mefile) | ||
| if filename.endswith("corr.mei"): | ||
| liberupdate.main(filename) | ||
| liberupdatev5.main(filename) | ||
| print(filename +" has been updated") | ||
| err= err+ (meichecker.main(filename[:-4]+"NEW2.mei")+"\n") |
There was a problem hiding this comment.
For something like this, a linter will generally prefer an f-string.
| err= err+ (meichecker.main(filename[:-4]+"NEW2.mei")+"\n") | |
| err += f"{meichecker.main(filename[:-4]}NEW2.mei)\n") |
|
Also, has this been formatted with |
|
@CThierrin Are you still trying to merge this? |
Fixes issues #8 (remove commented code from scripts) and #7 (directory is no longer hardcoded)