Opened 3 years ago

Closed 3 years ago

#24254 closed enhancement (fixed)

remove is_coercion_cached / is_conversion_cached methods

Reported by: vdelecroix Owned by:
Priority: major Milestone: sage-8.1
Component: coercion Keywords:
Cc: SimonKing, nthiery Merged in:
Authors: Vincent Delecroix Reviewers: Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: f6af60e (Commits) Commit: f6af60e5c18aeee3c2e3ffbc8b7c66a78b75656a
Dependencies: Stopgaps:

Description (last modified by vdelecroix)

Parent.is_coercion_cached and Parent.is_conversion_cached are:

  • not documented
  • trivial (return domain in self._coerce_from_hash)
  • useless for most users
  • appear in one place in the code of catgories.unital_algebras.UnitalAlgebras and two doctests

This ticket stands for hiding them (ie let their name starts with an underscore) and document them.

Change History (9)

comment:1 Changed 3 years ago by vdelecroix

  • Cc SimonKing added

comment:2 Changed 3 years ago by vdelecroix

  • Cc nthiery added
  • Description modified (diff)

comment:3 follow-up: Changed 3 years ago by tscrim

Overall, I agree, but self._coerce_from_hash is not public nor Python accessible. So you could not simply replace the call in unital algebras. So I think we should document them and make them hidden functions.

comment:4 in reply to: ↑ 3 Changed 3 years ago by SimonKing

Replying to tscrim:

Overall, I agree, but self._coerce_from_hash is not public nor Python accessible. So you could not simply replace the call in unital algebras. So I think we should document them and make them hidden functions.

I agree with that suggestion. I.e., do not remove it completely, but hide and document it.

comment:5 Changed 3 years ago by vdelecroix

  • Description modified (diff)

Thanks for your comments. I updated the description of the ticket accordingly.

comment:6 Changed 3 years ago by nthiery

For whatever it's worth, I am all for the current plan.

comment:7 Changed 3 years ago by vdelecroix

  • Authors set to Vincent Delecroix
  • Branch set to u/vdelecroix/24254
  • Commit set to f6af60e5c18aeee3c2e3ffbc8b7c66a78b75656a
  • Status changed from new to needs_review

New commits:

​f6af60e24254: hide and document is_coercion_cached/is_conversion_cached

comment:8 Changed 3 years ago by tscrim

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

LGTM.

comment:9 Changed 3 years ago by vbraun

  • Branch changed from u/vdelecroix/24254 to f6af60e5c18aeee3c2e3ffbc8b7c66a78b75656a
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.