move algebraic subschemes of toric varieties to their rightful places

[vbraun]

We should follow affine and projective space and put the implementation of algebraic subschemes into algebraic_scheme.py, the morphisms into morphism.py, and homsets into homset.py. The attached patch does this and adds documentation to the algebraic schemes module.

[novoselt]

I knew how affine/projective classes were packed into modules but deliberately violated the convention for toric varieties. Reasons:

1. Old modules are not documented/doctested, so it was easier to make sure that new stuff does follow requirements when it is in completely new files.
2. Old classes should be switched from old parents and methods to the new coercion framework and there may be some redesign in the process, so it is also good to keep them more separated.
3. Perhaps the most important: toric varieties/subvarieties/morphisms are more related to each other then to affine or projective analogs, in particular it makes more sense to group documentation on toric classes together rather than documentation on all kinds of ambients spaces, then all kinds of morphisms and so on. Note also that having classes in the same module simplifies Sphynx references to them - it is much more likely that documentation of a toric morphism will refer to toric varieties rather than affine morphisms. Also, I find it much more convenient to work with the source code files of toric framework as opposed to previous schemes. When there are 5 similar named classes with almost the same methods in a generically named file, I got confused which method I am editing.

So I am against this patch and would prefer instead to regroup older classes similar to toric varieties or perhaps split them into separate modules like affine_morphism, projective_morphism, etc. Of course, it is mostly just personal preference and taste, so if my reasoning is unconvincing I will consent...

[vbraun]

Hi Andrey,

I do understand your points and I do agree that there is a case to be made to separate them. But at least for now I would prefer to have everything in one place:

• Its easier to refactor things if you don't have to hunt for code in different files. If you get confused with similar-looking methods then that really shows that common code ought to be moved to the base class.

• From the end user point of view, there shouldn't be much of a difference between the affine schemes. Its an ambient space with equations. The end user is probably interested in presentation-independent properties like irreducible components, dimensions, singularities. Not so much about the details of the embedding. The module documentation should emphasize that that the different classes behave in the same way and not so much focus on the implementation details.

• Also, for "patches" of toric varieties we definitely need both the affine toric patches as well as the embedding in affine space (with the toric ideal). So again the different algebraic schemes end up being coupled more tightly than you think.

• Im unifying the somewhat duplicated methods projective_embedding vs. embedding_morphism in trac_10540_Spec_of_affine_toric_variety.patch.

In any case, I think we should first merge this sequence of patches and then figure out the coercion / category framework. If we can then split the code into different files without introducing cyclic dependencies then I'd be happy to do so.

[novoselt]

OK, let's proceed with it.

To make my life easier - did you make any changes to the code from/in toric_variety module except for hyperlinks adjustments?

[vbraun]

I didn't make any major changes to toric_variety.py, in this patch I just moved stuff to the algebraic schemes.

[novoselt]

I made some little changes to the documentation while I was reading it. If you agree with them, feel free to set this to positive review!

[vbraun]

Looks great, thanks!

[novoselt]

I am rebuilding to final 4.6.1 now, so I didn't check, but I am pretty sure that this patch will conflict with #9055 which got merged in 4.6.2.alpha0, so this one has to be rebased.

[novoselt]

Rebased agains sage-4.6.2.alpha0

[novoselt]

Indeed there was some fuzz, I have updated the original patch to apply smoothly.

Apply trac_10525_move_AlgebraicScheme_subscheme_toric.2.patch and trac_10525_reviewer.patch

[vbraun]

I still get fuzz, there is some issue with patch ordering I think. I've reordered my patch queue to reflect #9433 as closely as possible. See also  https://bitbucket.org/vbraun/mq-for-sage-toric-varieties

Rediffed patch follows.

Apply trac_10525_move_AlgebraicScheme_subscheme_toric.patch and trac_10525_reviewer.patch

[vbraun]

Updated patch

[vbraun]

For the patch bot: Depends on #9055

[novoselt]

Got it: fuzz in fano_toric_varieties was due to me having #10522 before this - somehow it got lost and I didn't take it into account when I rearranged my queue. Now Volker's latest version applies smoothly for me.