#23337 closed enhancement (fixed)

Use variable names instead of symbolic variables

Reported by: jdemeyer Owned by:
Priority: major Milestone: sage-8.0
Component: basic arithmetic Keywords:
Cc: SimonKing, rws Merged in:
Authors: Simon King, Ralf Stephan, Jeroen Demeyer Reviewers: Jeroen Demeyer, Travis Scrimshaw
Report Upstream: N/A Work issues:
Branch: ee4548f (Commits) Commit: ee4548f920444ef5739ccc10cb67d9ce73449d85
Dependencies: Stopgaps:

Description

Where possible, use QQ["x"] instead of QQ[x] to construct polynomial rings. After #10483, the latter will be deprecated.

Change History (15)

comment:1 Changed 23 months ago by jdemeyer

  • Branch set to u/jdemeyer/ticket/23337

comment:2 Changed 23 months ago by git

  • Commit set to d1f6fff086983e2224af518ac55faeec1593ff63

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

d1f6fffUse variable names instead of symbolic variables

comment:3 Changed 23 months ago by jdemeyer

  • Reviewers set to Jeroen Demeyer
  • Status changed from new to needs_review

comment:4 Changed 23 months ago by git

  • Commit changed from d1f6fff086983e2224af518ac55faeec1593ff63 to 52de325f447e0779bed655d0e57727c903b0f33e

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

52de325Use variable names instead of symbolic variables

comment:5 Changed 23 months ago by tscrim

There are some doctest failures (see patchbot). There are some actual changes in output type, but I am not sure if they are (more) correct or not. I would think some of them are actual regressions since they do not contain as much information (i.e., ZZ[x][y] has slightly different structure for things like factoring compared to ZZ[x,y]).

comment:6 Changed 23 months ago by jdemeyer

  • Status changed from needs_review to needs_work

comment:7 Changed 23 months ago by git

  • Commit changed from 52de325f447e0779bed655d0e57727c903b0f33e to ee4548f920444ef5739ccc10cb67d9ce73449d85

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

ee4548fUse variable names instead of symbolic variables

comment:8 Changed 23 months ago by jdemeyer

  • Status changed from needs_work to needs_review

I reverted the changes to flatten.py.

comment:9 Changed 23 months ago by jdemeyer

  • Status changed from needs_review to positive_review

comment:10 Changed 23 months ago by tscrim

  • Reviewers changed from Jeroen Demeyer to Jeroen Demeyer, Travis Scrimshaw
  • Status changed from positive_review to needs_work

I think I understand what is going wrong with flatten.py. It is checking the keys in D, which are actual variables and hence, do not compare with the string values. So the variable names all get included when constructing new_vars on this line:

new_vars = [[[t for t in v if t not in newD.keys()], b] for v,b in old_vars]

I think I see a good way to fix this. I also am going to do a slight bit of cleanup of that method.

comment:11 Changed 23 months ago by jdemeyer

Given that I reverted the changes to flatten.py and that the previously-failing doctests now pass, why was this set to needs_work?

comment:12 Changed 23 months ago by tscrim

  • Branch changed from u/jdemeyer/ticket/23337 to u/tscrim/ticket/var_names_not_symbolic_names-23337
  • Commit changed from ee4548f920444ef5739ccc10cb67d9ce73449d85 to e938684edebd1693f84db659f3bdeb05e4bfb5f8
  • Status changed from needs_work to needs_review

This fixes the failing doctests but addresses the problem in flatten.py (with some simple cleanup).


New commits:

e938684Handling flatten.py and some mild cleanup.

comment:13 Changed 23 months ago by tscrim

I would rather fix the problem than push it off until later.

comment:14 Changed 23 months ago by jdemeyer

  • Branch changed from u/tscrim/ticket/var_names_not_symbolic_names-23337 to u/jdemeyer/ticket/23337
  • Commit changed from e938684edebd1693f84db659f3bdeb05e4bfb5f8 to ee4548f920444ef5739ccc10cb67d9ce73449d85
  • Status changed from needs_review to positive_review

Creating tickets is cheap: #23343


New commits:

ee4548fUse variable names instead of symbolic variables

comment:15 Changed 22 months ago by vbraun

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