Opened 3 months ago

Closed 3 months ago

#30194 closed enhancement (fixed)

Extend FreeModule factory to construction of FiniteRankFreeModule and CombinatorialFreeModule

Reported by: mkoeppe Owned by:
Priority: major Milestone: sage-9.2
Component: linear algebra Keywords:
Cc: tscrim, nthiery, gh-mjungmath, egourgoulhon Merged in:
Authors: Matthias Koeppe Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: c23b9a4 (Commits) Commit: c23b9a4ee2407b29fbffb45ca432894abe962208
Dependencies: Stopgaps:

Description (last modified by mkoeppe)

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

Follow-up:

  • #30235: Add construction methods to FiniteRankFreeModule and CombinatorialFreeModule

Change History (30)

comment:1 Changed 3 months ago by mkoeppe

  • Description modified (diff)

comment:2 Changed 3 months ago by mkoeppe

  • Branch set to u/mkoeppe/extend_freemodule_factory_to_construction_of_finiterankfreemodule_and_combinatorialfreemodule

comment:3 Changed 3 months ago by git

  • Commit set to 549ebf33ca4e8a0b1d5b0ec353a4af1144513c65

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

549ebf3Create CombinatorialFreeModule if basis keys given

comment:4 Changed 3 months ago by git

  • Commit changed from 549ebf33ca4e8a0b1d5b0ec353a4af1144513c65 to 88391a27ddea1a678b43a055ce239c944f3eff01

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

88391a2CombinatorialFreeModule: Add construction

comment:5 Changed 3 months ago by git

  • Commit changed from 88391a27ddea1a678b43a055ce239c944f3eff01 to d7d1c14f6449cd49b0cb53fba99bbcf5af92ec4b

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

comment:6 Changed 3 months ago by git

  • Commit changed from d7d1c14f6449cd49b0cb53fba99bbcf5af92ec4b to 6d0fe39a0bccb4e3a4e8ada35edbb2bd632336ce

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

comment:7 Changed 3 months ago by mkoeppe

  • Authors set to Matthias Koeppe
  • Status changed from new to needs_review

comment:8 Changed 3 months ago by git

  • Commit changed from 6d0fe39a0bccb4e3a4e8ada35edbb2bd632336ce to 4d87237a056af85217ea03ac502314475348386f

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

comment:9 follow-up: Changed 3 months ago by tscrim

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.

comment:10 in reply to: ↑ 9 Changed 3 months ago by mkoeppe

Replying to tscrim:

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

Yes, will do.

comment:11 Changed 3 months ago by git

  • Commit changed from 4d87237a056af85217ea03ac502314475348386f to 47dce47c589f319ad3fc711b0c6c06281a8aaf10

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

47dce47Extend VectorSpace factory in the same way

comment:12 Changed 3 months ago by git

  • Commit changed from 47dce47c589f319ad3fc711b0c6c06281a8aaf10 to 93c8b7910c6fb60322d9c3eb31dbf9b45ee924bd

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

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

comment:13 Changed 3 months ago by mkoeppe

  • Status changed from needs_review to needs_work

Documentation needs more work

comment:14 Changed 3 months ago by git

  • Commit changed from 93c8b7910c6fb60322d9c3eb31dbf9b45ee924bd to d0c65b4fae5c411bf473bd3a6d566d3f860b1e6a

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

d0c65b4More documentation

comment:15 Changed 3 months ago by mkoeppe

  • Description modified (diff)
  • Status changed from needs_work to needs_review

comment:16 follow-up: Changed 3 months ago by tscrim

The construction() will need a doctest.

comment:17 Changed 3 months ago by git

  • Commit changed from d0c65b4fae5c411bf473bd3a6d566d3f860b1e6a to 78d063341d140193991bf2f273246d5005c1ea60

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

comment:18 Changed 3 months ago by mkoeppe

  • Description modified (diff)

comment:19 in reply to: ↑ 16 Changed 3 months ago by mkoeppe

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.

comment:20 Changed 3 months ago by tscrim

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

comment:21 Changed 3 months ago by mkoeppe

Thanks, I'll fix this

comment:22 Changed 3 months ago by git

  • Commit changed from 78d063341d140193991bf2f273246d5005c1ea60 to 1a07ec47a9d516905d6f5cbec2dfcb9d16a190d4

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

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

comment:23 Changed 3 months ago by git

  • Commit changed from 1a07ec47a9d516905d6f5cbec2dfcb9d16a190d4 to 0aec972e984b460cc385a535c2036624d35906f7

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

comment:24 follow-up: Changed 3 months ago by 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?

comment:25 in reply to: ↑ 24 Changed 3 months ago by egourgoulhon

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.

comment:26 Changed 3 months ago by git

  • Commit changed from 0aec972e984b460cc385a535c2036624d35906f7 to d2934507811e561606ec6ecbdd8bfcb4a2b06d56

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

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

comment:27 Changed 3 months ago by tscrim

  • Reviewers set to Travis Scrimshaw
  • Status changed from needs_review to positive_review

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.

comment:28 Changed 3 months ago by git

  • Commit changed from d2934507811e561606ec6ecbdd8bfcb4a2b06d56 to c23b9a4ee2407b29fbffb45ca432894abe962208
  • Status changed from positive_review to needs_review

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

c23b9a4Fix doc formatting

comment:29 Changed 3 months ago by mkoeppe

  • Status changed from needs_review to positive_review

Thanks!

comment:30 Changed 3 months ago by vbraun

  • Branch changed from u/mkoeppe/extend_freemodule_factory_to_construction_of_finiterankfreemodule_and_combinatorialfreemodule to c23b9a4ee2407b29fbffb45ca432894abe962208
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.