-
Notifications
You must be signed in to change notification settings - Fork 114
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Python run script #38
base: master
Are you sure you want to change the base?
Conversation
modularized generate_configuration, and updated Dockerfile and run.sh to work with the new run.py script
The concept of run.py was what I was thinking as well as I worked through minimizing win / nix differences since python is shared code. It leaves only docker to have to be managed separately. Unfortunately the whole program is a mess of globals which is definitely a carryover of its original form that I never managed to detangle. I think that is why you may have issues with the run.py. I'm sure it can work, but it will likely take a bit of hackery. Unfortunately this looks like a week I won't have much time to tackle this, so I am going to ensure do a few bug fixes to make it stable and start a dev branch to continue this. Hopefully I can devote a bit more time this weekend or later this week. |
I just took the time to search/replace all the global variable. I was able to get a case to generate, and the output looks ok. But a test suite would definitely be helpful. Still todo:
|
I've taken care of all the warnings about unknown symbols, except for on line #3995 I'm not sure what those variables are supposed to be... |
Wow, those snuck in there. I was going to look at a wedged base and must have immediately got sidetracked. I'll strip that when I get a chance. It shouldn't be there until it actually does something. |
Looking through this, I am going to bring most of it in. I should have converted to the dictionary references a long time ago, but just never go around to it. The global referencing was out of control. The are two areas of concern: Simplifying the workflow. I think I like the run.py approach since it keeps it os agnostic, but I am going to load your repo and try it out to make sure it make sense and can help reduce the execution paths. Ideally, I think I want 1 path that has 3 layers: Directly running the files, running through CLI, and docker execution through CLI. I will try that out with your setup. I think it gets close if I can just sort out the best docker approach to align. Importing both cadquery and solid at the top. I was specifically shielding the imports such that you could run without both libraries installed. People tend to have issues with installing one or the other and I was trying to prevent people from having to get both operational. This one I will probably adopt some of the general aspect, but hide the imports again so you can run with only one library working. |
I was actually thinking about the engine imports, and wanted to abstract away the engine-specific code, in the same vein as the engine specific helpers. I haven't had time to dig into the code enough to really understand the actual model generation, so I'm not sure if it's feasible, but I wanted to also look into completely abstracting away each specific section of a model. Something like a PlateGenerator class, that could have sub-class implementations for each engine. If it's possible, a similar approach could be used for the thumb cluster, where instead of lots of conditionals in the main generator file, the correct ThumbGenerator (default, mini, trackball, etc.) could be used. Again, I don't have much of a grasp on the actual model generation, so I'm not sure how feasible it is. On another note, I was also thinking about asymmetric builds. I was thinking about removing the "symmetry" setting in the config and instead adding an "asymmetries" dictionary. If the "asymmetries" setting is missing or empty, the build is symmetric. But if it contains any values, they would be used as overrides when generating the other side of the keyboard. It would allow a lot more portable customization of a complete build. All of this can be done in different updates, but I wanted to mention it here. If you think they might be useful ideas, it could help to keep them in mind as you integrate what you like from the PR. |
import getopt, sys | ||
import src.argument_parser as parser | ||
|
||
args = parser.parse() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry to ask, but have you considered to use internal python lib argparse instead of re implementing this?
RE: #35 (comment)
I started implementing what I had mentioned in the referenced comment. This is definitely a WIP, but I thought it might help explain what I was talking about.
The dactyl_manuform.py file is definitely not working, but I think it's enough to demonstrate what I was thinking. Essentially I made a global
shape_config
that can be referenced from the functions. I haven't really gotten into this yet, but I imagine I could also use thedebug
flag from the command line to replacedebug_exports
anddebug_trace