Skip to content

Conversation

@adam-urbanczyk
Copy link
Member

centerOption default value will change from "CenterOfMass" to "ProjectedOirigin"

Related to #202

@adam-urbanczyk adam-urbanczyk requested a review from jmwright April 7, 2020 16:22
@codecov
Copy link

codecov bot commented Apr 7, 2020

Codecov Report

Merging #313 into master will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #313      +/-   ##
==========================================
+ Coverage   95.36%   95.38%   +0.01%     
==========================================
  Files          18       19       +1     
  Lines        4685     4701      +16     
==========================================
+ Hits         4468     4484      +16     
  Misses        217      217              
Impacted Files Coverage Δ
cadquery/cq.py 94.00% <100.00%> (+0.01%) ⬆️
cadquery/utils.py 100.00% <100.00%> (ø)

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 d17d27d...efc9b12. Read the comment docs.

@jmwright
Copy link
Member

jmwright commented Apr 7, 2020

I think the code looks good. I do have a few questions.

  1. What's the next release that is mentioned in the warning? Does that include RC releases, or just the first stable release?
  2. I only get the warning on the first call of workplane. I'm assuming this is normal behavior, but I wanted to confirm that, and to make sure that's the intended behavior. I'm fine with it if it is.
  3. I get the following output:
>>> import cadquery as cq
>>> result = cq.Workplane("XY").workplane().box(1, 1, 1)
/home/jwright/Downloads/repos/cadquery/cadquery/utils.py:21: FutureWarning: Default walue of centerOption will change in next relase to ProjectedOrigin
  FutureWarning,

Why does the last FutureWarning have a trailing comma? Is there supposed to be more output after that?

@adam-urbanczyk
Copy link
Member Author

I think the code looks good. I do have a few questions.

1. What's the next release that is mentioned in the warning? Does that include RC releases, or just the first stable release?

I'd actually propose to release 2.0 soon, and change this in 2.1.

2. I only get the warning on the first call of `workplane`. I'm assuming this is normal behavior, but I wanted to confirm that, and to make sure that's the intended behavior. I'm fine with it if it is.

This is your local configuration I think - I cannot reproduce it.

3. I get the following output:
>>> import cadquery as cq
>>> result = cq.Workplane("XY").workplane().box(1, 1, 1)
/home/jwright/Downloads/repos/cadquery/cadquery/utils.py:21: FutureWarning: Default walue of centerOption will change in next relase to ProjectedOrigin
  FutureWarning,

Why does the last FutureWarning have a trailing comma? Is there supposed to be more output after that?

I'm afraid this is the default (not so nice) formatting of the warning message showing the line of code where the warnining is invoked. I'd propose to leave as-is.

@jmwright
Copy link
Member

jmwright commented Apr 7, 2020

Sounds good. Do you want to merge, or should I?

@adam-urbanczyk
Copy link
Member Author

I'll merge it.

@adam-urbanczyk adam-urbanczyk merged commit 6ecb0fc into master Apr 8, 2020
@jmwright jmwright deleted the adam-urbanczyk-origin-option branch October 28, 2025 20:14
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