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

Change to ProjectedOrigin #532

Merged
merged 8 commits into from Dec 8, 2020
Merged

Change to ProjectedOrigin #532

merged 8 commits into from Dec 8, 2020

Conversation

adam-urbanczyk
Copy link
Member

@adam-urbanczyk adam-urbanczyk commented Dec 4, 2020

Resolves #314

@adam-urbanczyk adam-urbanczyk changed the title Change to ProjectedOrigin [WIP] Change to ProjectedOrigin Dec 6, 2020
@adam-urbanczyk
Copy link
Member Author

It should pass now. In principle after merging this we could release (see: https://github.com/CadQuery/cadquery/projects/4), but maybe we want to merge other open PRs too.

@codecov
Copy link

codecov bot commented Dec 6, 2020

Codecov Report

Merging #532 (607f5d7) into master (cf275b0) will increase coverage by 0.14%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #532      +/-   ##
==========================================
+ Coverage   93.94%   94.09%   +0.14%     
==========================================
  Files          30       29       -1     
  Lines        6115     7160    +1045     
  Branches      640      904     +264     
==========================================
+ Hits         5745     6737     +992     
- Misses        235      257      +22     
- Partials      135      166      +31     
Impacted Files Coverage Δ
cadquery/cq.py 90.39% <ø> (+1.46%) ⬆️
tests/__init__.py 88.23% <100.00%> (+0.73%) ⬆️
tests/test_cadquery.py 99.04% <100.00%> (ø)
tests/test_selectors.py 100.00% <100.00%> (ø)
tests/test_workplanes.py 99.30% <0.00%> (-0.70%) ⬇️
cadquery/utils.py
cadquery/occ_impl/shapes.py 93.18% <0.00%> (+1.86%) ⬆️

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 cf275b0...607f5d7. Read the comment docs.

@adam-urbanczyk
Copy link
Member Author

Still need to fix some examples it seems.

@jmwright
Copy link
Member

jmwright commented Dec 6, 2020

@adam-urbanczyk These changes look ok, but I can take another look once the doc tests are passing if you want.

@adam-urbanczyk
Copy link
Member Author

Ok ready with all fixes.

Copy link
Member

@jmwright jmwright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adam-urbanczyk I made a few comments. The PR looks good. Thanks for fixing the selector descriptions too.

cadquery/cq.py Show resolved Hide resolved
doc/examples.rst Show resolved Hide resolved
tests/__init__.py Show resolved Hide resolved
@adam-urbanczyk
Copy link
Member Author

OK, I'm going ahead with the merge.

@adam-urbanczyk adam-urbanczyk merged commit a03648c into master Dec 8, 2020
@bernhard-42
Copy link
Contributor

@adam-urbanczyk Could it be that this breaks the assembly tutorial? It seems to work again when one e.g. adds centerOption to the second workplane call in make_connector:

def make_connector():

    rv = (
        cq.Workplane()
        .box(20, 20, 20)
        .faces("<X")
        .workplane()
        .cboreHole(6, 15, 18)
        .faces("<Z")
        .workplane(centerOption="CenterOfMass")
        .cboreHole(6, 15, 18)
    )

    # tag mating faces
    rv.faces(">X").tag("X").end()
    rv.faces(">Z").tag("Z").end()

    return rv

@adam-urbanczyk
Copy link
Member Author

Yest it does, thanks @bernhard-42 !

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.

Change centerOption to ProjectedOrigin and remove warning
3 participants