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

Extend FreeModule factory to construction of FiniteRankFreeModule and CombinatorialFreeModule #30194

Closed
mkoeppe opened this issue Jul 21, 2020 · 44 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Jul 21, 2020

This is to improve discoverability of these implementations of free modules.

Follow-up:

CC: @tscrim @nthiery @mjungmath @egourgoulhon

Component: linear algebra

Author: Matthias Koeppe

Branch/Commit: c23b9a4

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/30194

@mkoeppe mkoeppe added this to the sage-9.2 milestone Jul 21, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 21, 2020

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 21, 2020

Commit: 549ebf3

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 21, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

549ebf3Create CombinatorialFreeModule if basis keys given

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Changed commit from 549ebf3 to 88391a2

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 22, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

88391a2CombinatorialFreeModule: Add construction

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2020

Changed commit from 88391a2 to d7d1c14

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

2a81585Draft
1fe1d3cCreate CombinatorialFreeModule if basis keys given
d7d1c14CombinatorialFreeModule: Add construction

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

69fea07zero_vector: Handle bad input like zero_vector(3.6) before passing it to ZZ.__pow__
6d0fe39FreeModule: SImplify argument handling

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2020

Changed commit from d7d1c14 to 6d0fe39

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 26, 2020

Author: Matthias Koeppe

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2020

Changed commit from 6d0fe39 to 4d87237

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 26, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

4d87237src/sage/misc/sageinspect.py: Do not use FreeModule as an example for a class instance

@tscrim
Copy link
Collaborator

tscrim commented Jul 27, 2020

comment:9

I am happy with this change, but a lot more documentation is needed. Likely you will want to move most of the doc from the FreeModuleFactory.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 27, 2020

comment:10

Replying to @tscrim:

Likely you will want to move most of the doc from the FreeModuleFactory.

Yes, will do.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

47dce47Extend VectorSpace factory in the same way

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2020

Changed commit from 4d87237 to 47dce47

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2020

Changed commit from 47dce47 to 93c8b79

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

33f0d37Move documentation from FreeModuleFactory to FreeModule
93c8b79FreeModule: Add to documentation

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 27, 2020

comment:13

Documentation needs more work

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

d0c65b4More documentation

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 27, 2020

Changed commit from 93c8b79 to d0c65b4

@mkoeppe

This comment has been minimized.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 28, 2020

Changed commit from d0c65b4 to 78d0633

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 28, 2020

Branch pushed to git repo; I updated commit sha1. This was a forced push. New commits:

9ee2a48Create CombinatorialFreeModule if basis keys given
d08ee98zero_vector: Handle bad input like zero_vector(3.6) before passing it to ZZ.__pow__
5d56004FreeModule: SImplify argument handling
9116a1csrc/sage/misc/sageinspect.py: Do not use FreeModule as an example for a class instance
20b10f0Extend VectorSpace factory in the same way
6af7fa4Move documentation from FreeModuleFactory to FreeModule
5910876FreeModule: Add to documentation
78d0633More documentation

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 28, 2020

comment:19

Replying to @tscrim:

The construction() will need a doctest.

I have taken out the commit that adds it; I will continue with that on #30235.

@tscrim
Copy link
Collaborator

tscrim commented Jul 30, 2020

comment:20

Doctest failures in src/sage/structure/factory.pyx according to the patchbot.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Jul 30, 2020

comment:21

Thanks, I'll fix this

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

1a07ec4sage.structure.factory: Update doctests that used FreeModule

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Jul 30, 2020

Changed commit from 78d0633 to 1a07ec4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2020

Changed commit from 1a07ec4 to 0aec972

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 3, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

0aec972Merge tag '9.2.beta7' into t/30194/extend_freemodule_factory_to_construction_of_finiterankfreemodule_and_combinatorialfreemodule

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 3, 2020

comment:24

patchbot plugin complains about this:

+       EXAMPLE::
+       EXAMPLE::
+       EXAMPLE::

These are headers for a single example each. Should this be changed, or do we just ignore this plugin message?

@egourgoulhon
Copy link
Member

comment:25

Replying to @mkoeppe:

patchbot plugin complains about this:

+       EXAMPLE::
+       EXAMPLE::
+       EXAMPLE::

These are headers for a single example each. Should this be changed, or do we just ignore this plugin message?

I think the convention is to always use EXAMPLES (plural) even for a single example, hence the patchbot complaint.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 4, 2020

Changed commit from 0aec972 to d293450

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 4, 2020

Branch pushed to git repo; I updated commit sha1. New commits:

d293450src/sage/modules/free_module.py: EXAMPLE -> EXAMPLES

@tscrim
Copy link
Collaborator

tscrim commented Aug 7, 2020

comment:27

Thank you. One minor nitpick:

-        TODO: replace the above by ``TestSuite(...).run()``, once
-        :meth:`_test_pickling` will test unique representation and not
-        only equality.
+        .. TODO::
+
+            Replace the above by ``TestSuite(...).run()``, once
+            :meth:`_test_pickling` will test unique representation
+            and not only equality.

Once changed, you can set a positive review on my behalf.

@tscrim
Copy link
Collaborator

tscrim commented Aug 7, 2020

Reviewer: Travis Scrimshaw

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 7, 2020

Changed commit from d293450 to c23b9a4

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 7, 2020

Branch pushed to git repo; I updated commit sha1 and set ticket back to needs_review. New commits:

c23b9a4Fix doc formatting

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 7, 2020

comment:29

Thanks!

@vbraun
Copy link
Member

vbraun commented Aug 10, 2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants