[ToricVarieties] Add total_space(E) #2781
Conversation
Codecov Report
@@ Coverage Diff @@
## master #2781 +/- ##
==========================================
- Coverage 73.63% 73.61% -0.02%
==========================================
Files 455 455
Lines 64411 64419 +8
==========================================
- Hits 47427 47421 -6
- Misses 16984 16998 +14
|
| X = total_space(canonical_bundle(S)) | ||
| @test is_smooth(X) == true | ||
| @test !is_fano(X) | ||
| @test !is_complete(X) |
There was a problem hiding this comment.
Indention is weird here. I think you are mixing tabs and spaces. Please do not use tabs. In fact, please ideally read our notes about code formatting in OSCAR 😄
There was a problem hiding this comment.
Thank you very much! I corrected the commit following your remark.
There was a problem hiding this comment.
It seemed that there was still a mixture of tabs and spaces. If the editorconfig is correctly acknowledged by VSCode, pressing tab should add two white space characters. (For me, they show as two centered dots, whereas a tab is shown as an arrow to the right).
I have taken the liberty to update your branch with a change, that turns all tabs into two whitespace characters. If I did not make a mistake, this should now agree with the requirements in https://docs.oscar-system.org/dev/DeveloperDocumentation/styleguide/#Code-formatting.
There was a problem hiding this comment.
I am very sorry @mgemath , in my attempt to help I must have overwritten part (hopefully just a small part) of your desired changes. Really sorry. I just tried to fix my mistake, but could not. Maybe you immediately see what change I overwrote?
There was a problem hiding this comment.
Do not worry @HereAround. You were trying to correct my own mistakes. So it is me who must apologize in the first place. Anyway, it seems that you undone some of my corrections. This is the list.
file: src/AlgebraicGeometry/ToricVarieties/Proj/constructors.jl
lines: 28, 32, 39, 43, 60, 65.
file: src/exports.jl
line: 1378.
file: test/AlgebraicGeometry/ToricVarieties/proj.jl
line: 43.
You can find the changes here: https://github.com/oscar-system/Oscar.jl/compare/8eb7eb4cfc9d1aca1f80cbbe76f129b5987e6d06..b68e44e915573330cea170b104528d7a5cb8c595
There was a problem hiding this comment.
Thank you @mgemath. Just corrected my earlier mistake and added one more commit on top with a few minor optimizations. Let us see if the tests now pass.
HereAround
force-pushed
the
TotalSpace
branch
from
8eb7eb4
to
b68e44e
Compare
HereAround
force-pushed
the
TotalSpace
branch
from
b68e44e
to
8b90f53
Compare
HereAround
force-pushed
the
TotalSpace
branch
from
8b90f53
to
2c74f72
Compare
There was a problem hiding this comment.
Thank you @gmemath. This looks good to me.
As discussed above, I adjusted the tabs vs whitespaces issue. We can discuss more on slack if this remains to be an issue. I have also done a few minor adjustments - nothing major.
Long story short: Thank you for the added functionality. Great!
Here we add the function total_space(E::ToricLineBundle...).
It return, as a toric variety, the total space of the sum of the line bundles appearing in E.