-
-
Notifications
You must be signed in to change notification settings - Fork 487
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
sage.geometry
: Add some # optional
, reformat doctests
#35586
sage.geometry
: Add some # optional
, reformat doctests
#35586
Conversation
sage: p600 = polytopes.six_hundred_cell(exact=True) # not tested - very long time # optional - sage.groups | ||
sage: len(list(p600.bounded_edges())) # not tested - very long time # optional - sage.groups | ||
sage: p600 = polytopes.six_hundred_cell(exact=True) # not tested - very long time, optional - sage.groups | ||
sage: len(list(p600.bounded_edges())) # not tested - very long time, optional - sage.groups | ||
720 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Now we have two formats # ... # optional - ...
, # ..., optional - ...
for the same thing?
I prefer the second format (changed mind after second look).
So, OK.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm using the second format here because there were no alignment opportunities.
Otherwise, except Build & Test, it looks good to me. |
Now tests are passing |
@@ -1684,7 +1684,7 @@ def _contains(self, point, region='whole cone'): | |||
sage: c = Cone([(1,0), (0,1)]) | |||
sage: c._contains((1,I)) # optional - sage.symbolic | |||
False | |||
sage: c._contains(vector(QQbar, [1,I])) # optional - sage.symbolic | |||
sage: c._contains(vector(QQbar, [1,I])) # optional - sage.rings.number_field | |||
False |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Don't you need both?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You are right, thanks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in 2be9942
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks.
Thanks for reviewing! |
Documentation preview for this PR (built with commit 2be9942) is ready! 🎉 |
📚 Description
Update
# optional
, PEP 8 fixes for doctests, reformat/align# optional
.Taken from (and tested with) current #35095
Part of:
📝 Checklist
⌛ Dependencies