Skip to content

Python run script#38

Open
itised wants to merge 10 commits into
joshreve:masterfrom
itised:python-run-script
Open

Python run script#38
itised wants to merge 10 commits into
joshreve:masterfrom
itised:python-run-script

Conversation

@itised

@itised itised commented Aug 23, 2021

Copy link
Copy Markdown

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 the debug flag from the command line to replace debug_exports and debug_trace

itised added 3 commits August 23, 2021 12:42
modularized generate_configuration, and updated Dockerfile and run.sh to work with the new run.py script
@joshreve

Copy link
Copy Markdown
Owner

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.

@itised

itised commented Aug 24, 2021

Copy link
Copy Markdown
Author

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:

  • update run.bat
  • update README
  • I'm sure much more that I'm just blanking out on now...

@itised

itised commented Aug 24, 2021

Copy link
Copy Markdown
Author

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...

@joshreve

Copy link
Copy Markdown
Owner

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.

@joshreve

Copy link
Copy Markdown
Owner

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.

@itised

itised commented Aug 31, 2021

Copy link
Copy Markdown
Author

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.

@joshreve
joshreve marked this pull request as ready for review September 4, 2021 19:48
Comment thread run.py
import getopt, sys
import src.argument_parser as parser

args = parser.parse()

Copy link
Copy Markdown

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?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants