#30436 closed enhancement (fixed)

some fixes suggested by lgtm

Reported by: chapoton Owned by:
Priority: minor Milestone: sage-9.2
Component: refactoring Keywords:
Cc: tscrim Merged in:
Authors: Frédéric Chapoton Reviewers: Marc Mezzarobba
Report Upstream: N/A Work issues:
Branch: d9ed254 (Commits, GitHub, GitLab) Commit: d9ed254ebfdd95ffb3c757a0ef8e7a49e8d3448f
Dependencies: Stopgaps:

Status badges

Description

about unused imports and variables, etc

Change History (7)

comment:1 Changed 13 months ago by chapoton

  • Branch set to u/chapoton/30436
  • Commit set to 66575371303815a366c0c6e6dadbf78714b62a91
  • Status changed from new to needs_review

New commits:

6657537some fixes suggested by lgtm (unused variables and imports, etc)

comment:2 Changed 13 months ago by git

  • Commit changed from 66575371303815a366c0c6e6dadbf78714b62a91 to d9ed254ebfdd95ffb3c757a0ef8e7a49e8d3448f

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

d9ed254some fixes suggested by lgtm (unused variables and imports, etc)

comment:3 Changed 13 months ago by chapoton

  • Cc tscrim added

morally green bot, please review

comment:4 Changed 13 months ago by mmezzarobba

  • Reviewers set to Marc Mezzarobba

I understand neither the first of your changes nor the code it modifies. What is the point of calling AlternatingSignMatrix() and ignoring its result? Does it have side-effects? If not, why aren't you removing the instruction entirely (and the assignment to M on the previous line as well)?

The rest looks fine to me.

comment:5 Changed 13 months ago by chapoton

Well, this line checks that the matrix is indeed an alternating sign matrix. We do not care about the result, so there is not need to store it.

comment:6 Changed 13 months ago by mmezzarobba

  • Status changed from needs_review to positive_review

Ok, thanks.

comment:7 Changed 13 months ago by vbraun

  • Branch changed from u/chapoton/30436 to d9ed254ebfdd95ffb3c757a0ef8e7a49e8d3448f
  • Resolution set to fixed
  • Status changed from positive_review to closed
Note: See TracTickets for help on using tickets.