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

Create a child workplane on a vertex #480

Merged

Conversation

ArmoredBlood
Copy link
Contributor

This PR should resolve #24 and #149 (I think).

This is also my first time coding anything in the CAD domain, so I'm open to criticism on the solution since I'm not very familiar with the domain. I have an intermediate level of knowledge of CAD as a hobby user, not as a programmer.

I'm also new to CadQuery, so the only potential issue I see is this only looks at the immediate parent workplane for a Face to use. I'm not sure if it should instead do some kind of recursive search through the entire stack for a Face (assuming there's a long stack of related workplanes)

@ArmoredBlood
Copy link
Contributor Author

ArmoredBlood commented Oct 13, 2020

I'm not sure what mypy is wanting from me in the travisCI build. My logic should only be ran when the cq object is a Face (which has the normalAt() attribute.

@jmwright
Copy link
Member

The mypy stuff is still a mystery to me, but the signature in the lint error matches CQObject.

So maybe mypy at least thinks you're grabbing the wrong level of object? @adam-urbanczyk will know, I'm sure.

@codecov
Copy link

codecov bot commented Oct 14, 2020

Codecov Report

Merging #480 into master will increase coverage by 0.02%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #480      +/-   ##
==========================================
+ Coverage   93.63%   93.65%   +0.02%     
==========================================
  Files          30       30              
  Lines        5859     5880      +21     
  Branches      624      626       +2     
==========================================
+ Hits         5486     5507      +21     
  Misses        234      234              
  Partials      139      139              
Impacted Files Coverage Δ
cadquery/cq.py 88.34% <ø> (+0.06%) ⬆️
tests/test_cadquery.py 98.99% <ø> (+<0.01%) ⬆️
cadquery/occ_impl/shapes.py 90.94% <0.00%> (+0.04%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7fd45e0...110c9a4. Read the comment docs.

@adam-urbanczyk
Copy link
Member

adam-urbanczyk commented Oct 15, 2020

Thanks @ArmoredBlood . Mypy does not know that the result of val() has the same type between the two calls.

Looks good to me, @jmwright any thoughts?

@jmwright
Copy link
Member

+1 to merge

@adam-urbanczyk adam-urbanczyk merged commit cac5cf9 into CadQuery:master Oct 15, 2020
@jmwright
Copy link
Member

Thanks for the contribution @ArmoredBlood

@ArmoredBlood ArmoredBlood deleted the workplane-parent-plane-check branch October 15, 2020 20:18
marcus7070 pushed a commit to marcus7070/cadquery that referenced this pull request Oct 24, 2020
* add parent face detection to workplane()

* Update cq.py

* add nonetype check and tests

* fixed test formatting

* mypy fix attempt

Co-authored-by: Adam Urbańczyk <[email protected]>
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.

Workplane orientation breaks when "Locating workplane on a vertex".
3 participants