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

raise TypeError when Location param is a tuple #723

Merged
merged 1 commit into from
Apr 14, 2021

Conversation

RubenRubens
Copy link
Contributor

Let me know if this was what you had on mind @marcus7070.

PR requested on #717 .

@fedorkotov
Copy link
Contributor

(copying my comment from now closed #722)
I don't know for sure if @marcus7070 meant that tuple should be accepted or that exception should be raised when it is passed. But either way I think that if ... elif .. chain should end with else (not elif) which raises an exception when anything of unsupported type is passed.

@RubenRubens
Copy link
Contributor Author

RubenRubens commented Apr 13, 2021

Good point @fedorkotov. I have just choose to end with elif because this is a tuple specific issue. But it can end with an else as you said. Thanks

@codecov
Copy link

codecov bot commented Apr 13, 2021

Codecov Report

Merging #723 (9e413a2) into master (a71a93e) will decrease coverage by 0.01%.
The diff coverage is 94.07%.

❗ Current head 9e413a2 differs from pull request most recent head d5cb4c6. Consider uploading reports for the commit d5cb4c6 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master     #723      +/-   ##
==========================================
- Coverage   94.51%   94.50%   -0.02%     
==========================================
  Files          32       32              
  Lines        7078     7210     +132     
  Branches      768      786      +18     
==========================================
+ Hits         6690     6814     +124     
- Misses        254      261       +7     
- Partials      134      135       +1     
Impacted Files Coverage Δ
cadquery/occ_impl/geom.py 88.47% <64.28%> (-0.49%) ⬇️
cadquery/occ_impl/solver.py 90.55% <86.36%> (+0.64%) ⬆️
cadquery/assembly.py 91.07% <93.75%> (+0.53%) ⬆️
tests/test_assembly.py 100.00% <100.00%> (ø)
tests/test_cadquery.py 99.18% <100.00%> (+<0.01%) ⬆️
cadquery/cqgi.py 80.16% <0.00%> (-1.24%) ⬇️
tests/test_workplanes.py 99.30% <0.00%> (-0.70%) ⬇️
tests/test_examples.py 89.47% <0.00%> (-0.27%) ⬇️
cadquery/occ_impl/exporters/__init__.py 93.00% <0.00%> (-0.07%) ⬇️
... and 3 more

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 a71a93e...d5cb4c6. Read the comment docs.

Copy link
Member

@marcus7070 marcus7070 left a comment

Choose a reason for hiding this comment

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

Looks good, thanks @RubenRubens!

@marcus7070 marcus7070 requested a review from jmwright April 14, 2021 02:18
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.

Thanks @RubenRubens !

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.

4 participants