Skip to content
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

CAM: Fixed script-style postprocessors getting the wrong __name__ #18538

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

LarryWoestman
Copy link
Contributor

This fixes #18493

Script-style (legacy) postprocessors had a name of "script_module" instead of the postprocessor name. This value shows up in a comment in the generated g-code for most postprocessors. This causes confusion for the users of the postprocessors. This PR passes the postprocessor name to the WrapperPost class so it can be used when invoking the postprocessor.

Also removed a "syspath = sys.path" line because syspath wasn't being used anywhere in the code.

@LarryWoestman
Copy link
Contributor Author

I don't know how to do a "backport" but would be happy to learn :-)

This PR should be able to be applied to the 1.0 release tree without issues as far as I know.

@maxwxyz maxwxyz requested a review from sliptonic December 16, 2024 07:03
@maxwxyz
Copy link
Collaborator

maxwxyz commented Dec 16, 2024

@LarryWoestman we'll do the backport for a 1.0.1 bugfix release

@LarryWoestman
Copy link
Contributor Author

I was able to figure out a test for this issue, so added the test as well with the latest force-push.

@maxwxyz
Copy link
Collaborator

maxwxyz commented Dec 20, 2024

@LarryWoestman could you resolve the conflict in this PR?

@LarryWoestman
Copy link
Contributor Author

I would be happy to fix it if I could figure out what the conflict is. Can you be more detailed about what the problem is or what I can do to figure out what the problem is?

Thanks!

@maxwxyz
Copy link
Collaborator

maxwxyz commented Dec 20, 2024

@LarryWoestman
Copy link
Contributor Author

LarryWoestman commented Dec 20, 2024

I noticed that #18539 was already merged when it was supposed to wait until this issue was merged. That might be the cause of the conflict since the conflict isn't in code that I modified.
This conflict isn't in a file that I modified. The conflict isn't even in the CAM directory tree :-)
I don't see a conflict when I download the latest version of "main" to my "staging" branch.
Perhaps this is caused by the pre-commit-ci bot checkin?
I am currently in the process of building and testing a "staging" branch that is up to date with the main branch except for my changes. I will force push a new version of the "staging" branch as soon as that is done. It might take me until my evening since I have to go out with my wife :-)
Yes, the change is caused by the pre-commit-ci bot's change. I don't know what to do about that.

      Also added a test that the __name__ of the postprocessor is correct
@LarryWoestman
Copy link
Contributor Author

I have force pushed an up-to-date version of the "staging" branch.

@LarryWoestman
Copy link
Contributor Author

I don't see any pre-commit-ci bot changes to this force push, so perhaps the problem has gone away? :-)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Mod: CAM Related to the CAM/Path Workbench Needs backport Needs backport to 1.0.1
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CAM: script-style (legacy) postprocessors have a __name__ of "script_module" instead of the postprocessor name
3 participants