Add script to rename 3d models #205
Conversation
This does use the json file that would normally be used by the fix_footprints script for fixing footprint fields of symbols.
There was a problem hiding this comment.
Overall, I'm not sure if this script in its current functionality is something we really want: it doesn't rename the source files, and it doesn't change occurrences of the filename inside the 3D model files. @Shackmeister You seem like our main packages3D person to me, what do you think about this script?
Regardless, I've provided some changes I'd like to see before I'd merge the script in its current form.
|
|
||
| if args.replace: | ||
| with open(args.replace) as json_file: | ||
| replacements = json.loads(json_file.read()) |
There was a problem hiding this comment.
Use json.load(json_file) instead of reading the file into a string yourself and then loading the string.
| else: | ||
| replacements = {} | ||
|
|
||
| KEYS = ['library', 'footprint', 'prefix', "replace"] |
There was a problem hiding this comment.
Please use consistent quote marks within each file. You mix single and double quotes unnecessarily throughout this script, but this line in particular looks extra silly.
Moreover, it looks like only the footprint key is used by this script, so why bother creating the others if they're not present in the file?
|
|
||
| #print(glob.glob('{:}*.kicad_mod'.format(lib))) | ||
| for model_path in glob.glob('{:s}*.wrl'.format(lib)): | ||
| model_name = ntpath.basename(model_path) |
There was a problem hiding this comment.
Is there a good reason to use ntpath here instead of os.path? If not, please change it to os.path. Otherwise, please add a comment explaining why, to avoid this question coming up again in the future. 🙂
| import re | ||
| import json | ||
| import ntpath | ||
| import glob |
There was a problem hiding this comment.
Please import one module per line, and alphabetize this group of imports.
|
|
||
| parser.add_argument('-l', '--lib', nargs='+', help='3d model libraries (.3dshapes files)', action='store') | ||
| parser.add_argument('-r', '--replace', help='Path to JSON file containing replacement information') | ||
| parser.add_argument('-v', '--verbose', help='Verbosity level', action='count') |
There was a problem hiding this comment.
The --verbose option is carefully set up (though you should use default=0 when adding the argument instead of the if block on lines 22-23), but never used. Either remove the option or make the script actually do something with it.
| parser = argparse.ArgumentParser(description="Check symbols for footprint errors") | ||
|
|
||
| parser.add_argument('-l', '--lib', nargs='+', help='3d model libraries (.3dshapes files)', action='store') | ||
| parser.add_argument('-r', '--replace', help='Path to JSON file containing replacement information') |
There was a problem hiding this comment.
Both --lib and --replace are required in order for the script to do anything at all. In that case, I suggest you change them both to required positional arguments, making the usage something like:
rename.py [-h] replace lib [lib ...]
This would also remove the need for the conditional on lines 27-32: you'd only need the part inside the if block because args.replace would be guaranteed to be a string.
| import ntpath | ||
| import glob | ||
|
|
||
| parser = argparse.ArgumentParser(description="Check symbols for footprint errors") |
There was a problem hiding this comment.
Please set an accurate description for the ArgumentParser.
| @@ -0,0 +1,56 @@ | |||
| #!/usr/bin/env python | |||
| # -*- coding: utf-8 -*- | |||
There was a problem hiding this comment.
A module-level docstring briefly describing what this script is would be nice to have.
|
Personally I highly prefer to update the scripts used to generate the models in the first place. |
|
I agree with you for scripted models, but not all of our 3D models are scripted. I think this script could be useful for renaming models that were made by hand, but it would need to rename the .FCStd file as well, which it doesn't do in its current form. Also, STEP files have the 3D model name in them multiple times, and I don't know how important it is to keep this up-to-date with the new filename. This is really what I was asking about before. |
|
This script has never been intended to replace maintenance of the scripted 3d models. (It got used to speed up the process of fixing the lib for the v5 release.) |
This does use the json file that would normally be used by the
fix_footprints script for fixing footprint fields of symbols.