Opened 9 years ago

Closed 9 years ago

#13641 closed enhancement (fixed)

Short representation of morphisms

Reported by: caruso Owned by: nthiery
Priority: major Milestone: sage-5.5
Component: categories Keywords: representation of morphism
Cc: Merged in: sage-5.5.beta2
Authors: Xavier Caruso Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: Commit:
Dependencies: Stopgaps:

Status badges

Description (last modified by caruso)

A small patch implementing a function _repr_short which returns a short representation of a morphism. (The representation is e_1 |--> f_1, ..., e_n |-> f_n where $e_1, \ldots, e_n$ is a generating family of the source.)

This function is used in #13215

Apply trac_13641_short_repr_morphism.patch

Attachments (1)

trac_13641_short_repr_morphism.patch (1.8 KB) - added by caruso 9 years ago.

Download all attachments as: .zip

Change History (16)

comment:1 Changed 9 years ago by caruso

  • Authors set to caruso

comment:2 Changed 9 years ago by caruso

  • Status changed from new to needs_review

comment:3 Changed 9 years ago by tscrim

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

Few minor things:

  • You need 2 colons for examples -> EXAMPLES::
  • I'd like this as a public method, i.e. name it short_repr(). If you'd prefer to keep this private, please name it _repr_short().
  • Add a second example with multiple generators.
  • Add examples of the form (or do all examples in this form):
    sage: print f.short_repr()
    
    which will show the actual output of the string.
  • I'd like a blank line between the documentation and the EXAMPLES:: block.
  • You'll need to put in your real name for the ticket.

Thanks,
Travis

comment:4 Changed 9 years ago by caruso

  • Authors changed from caruso to Xavier Caruso
  • Status changed from needs_work to needs_review

Done.

comment:5 Changed 9 years ago by caruso

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

I just noticed that there is already:

  • one function _short_repr in sage/rings/function_field/maps.py
  • one function _short_repr in modular/etaproducts.py
  • one function _repr_short in modular/dirichlet.py
  • one function _repr_short in rings/ideal.py
  • one function _repr_short in rings/number_field/number_field_ideal.py
  • one function _repr_short in rings/polynomial/multi_polynomial_libsingular.pyx
  • one function _repr_short in rings/polynomial/plural.pyx

I think we should be coherent on the name. What do you propose?

comment:6 Changed 9 years ago by tscrim

Hmm...based on what's already in sage and that it is a variant on _repr_, I'd call it _repr_short() (akin to _repr_defn()).

Personally I don't like hiding alternative outputs, but that's an issue for another time.

Thanks,
Travis

comment:7 Changed 9 years ago by caruso

Fine. I've updated my patch.

comment:8 Changed 9 years ago by tscrim

  • Status changed from needs_info to needs_review

Looks good. Thank you.

comment:9 Changed 9 years ago by tscrim

  • Status changed from needs_review to positive_review

comment:10 Changed 9 years ago by jdemeyer

  • Status changed from positive_review to needs_work

The patch needs a proper commit message.

comment:11 Changed 9 years ago by caruso

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

comment:12 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to needs_work

The patch still needs a proper commit message. A commit message can be added with

hg qrefresh -e

before exporting the patch.

Changed 9 years ago by caruso

comment:13 Changed 9 years ago by caruso

  • Status changed from needs_work to needs_review

Ok, thanks. I think everything is fine, now.

comment:14 Changed 9 years ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:15 Changed 9 years ago by jdemeyer

  • Merged in set to sage-5.5.beta2
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.