Opened 4 years ago

Closed 4 years ago

# Refactor IntegerMulAction

Reported by: Owned by: jdemeyer major sage-8.2 coercion Jeroen Demeyer Travis Scrimshaw N/A a359862 a35986207005594e923ad97e12cb468d0885dfb4

Refactor IntegerMulAction in preparation for #24247:

1. Add an abstract base class IntegerAction which will also be used to implement IntegerPowAction in #24247.
1. Add a new helper function parent_is_integers to check if some type/parent represents the integers.
1. Instead of the hacks in discover_action and verify_action involving an explicit IntegerMulAction check, return an IntegerMulAction for anything satisfying parent_is_integers().

### comment:1 Changed 4 years ago by jdemeyer

• Branch set to u/jdemeyer/ticket/24467

### comment:2 Changed 4 years ago by jdemeyer

• Commit set to 85cabe71cb8a3f034b99a71e65c5329b79c2a8b7
• Status changed from new to needs_review

New commits:

 ​85cabe7 Refactor IntegerMulAction

### comment:3 Changed 4 years ago by git

• Commit changed from 85cabe71cb8a3f034b99a71e65c5329b79c2a8b7 to a2d60efa250eb7806f67f313738d67b3288577d3

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

 ​a2d60ef Also verify IntegerAction instances

### comment:4 Changed 4 years ago by jdemeyer

• Description modified (diff)

### comment:5 follow-up: ↓ 6 Changed 4 years ago by tscrim

• Reviewers set to Travis Scrimshaw

The · is not ascii nor latex: n · a = a + a + ... + a. So if you change that to \cdot (personally, I would also change the ... to \cdots), then you can set a positive review on my behalf.

### comment:6 in reply to: ↑ 5 ; follow-up: ↓ 9 Changed 4 years ago by jdemeyer

The · is not ascii nor latex

Is that a problem really? The documentation builds fine and I think that · is more readable than \cdot in text mode.

### comment:7 Changed 4 years ago by git

• Commit changed from a2d60efa250eb7806f67f313738d67b3288577d3 to a35986207005594e923ad97e12cb468d0885dfb4

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

 ​a359862 Use \cdot instead of ·

### comment:8 Changed 4 years ago by jdemeyer

• Status changed from needs_review to positive_review

### comment:9 in reply to: ↑ 6 ; follow-up: ↓ 10 Changed 4 years ago by tscrim

The · is not ascii nor latex

Is that a problem really? The documentation builds fine and I think that · is more readable than \cdot in text mode.

With the non-ascii characters, the pdf docs tend to have difficulty if the encoding is not declared (in particular, the non-ascii hyphen comes to mind). What we probably should do is add a bunch of the unicode versions of certain latex commands to the console doc replacer since we generally assume terminals support unicode.

Thanks for the change.

### comment:10 in reply to: ↑ 9 Changed 4 years ago by jdemeyer

What we probably should do is add a bunch of the unicode versions of certain latex commands to the console doc replacer since we generally assume terminals support unicode.

No, we do the opposite: replace certain Unicode characters by latex when building the PDF docs. In src/doc/common/conf.py you will find for example

\DeclareUnicodeCharacter{00B7}{\cdot}


saying that · should be replaced by \cdot. So · in docstrings should work just fine.

### comment:11 Changed 4 years ago by tscrim

I see. I wasn't aware of that. Thank you.

### comment:12 Changed 4 years ago by vbraun

• Branch changed from u/jdemeyer/ticket/24467 to a35986207005594e923ad97e12cb468d0885dfb4
• Resolution set to fixed
• Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.