Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Replace NumberField_generic.__latex_variable_name by ._latex_names #30372

Closed
mkoeppe opened this issue Aug 15, 2020 · 14 comments
Closed

Replace NumberField_generic.__latex_variable_name by ._latex_names #30372

mkoeppe opened this issue Aug 15, 2020 · 14 comments

Comments

@mkoeppe
Copy link
Contributor

mkoeppe commented Aug 15, 2020

as observed in #30360,

sage: K3.<a> = NumberField(x^3+x^2+1, latex_name=['alpha'])                                                                         
sage: K3.latex_variable_names()                                                                                                     
['a']
sage: K3.latex_variable_name()                                                                                                      
'alpha'

We fix this by changing the attribute __latex_variable_name, only used in NumberField_generic, to the widely used _latex_names.

We also deprecate the method latex_variable_name, which is also only used in sage.rings.number_field, changing its uses to latex_variable_names.

CC: @kliem @videlec @tscrim

Component: number fields

Author: Matthias Koeppe

Branch/Commit: da551dc

Reviewer: Travis Scrimshaw

Issue created by migration from https://trac.sagemath.org/ticket/30372

@mkoeppe mkoeppe added this to the sage-9.2 milestone Aug 15, 2020
@mkoeppe

This comment has been minimized.

@mkoeppe

This comment has been minimized.

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 15, 2020

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 15, 2020

Author: Matthias Koeppe

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 15, 2020

New commits:

1842e47sage.rings.number_field.number_field.NumberField_generic: Use _latex_names instead of __latex_variable_name, deprecate .latex_variable_name()

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 15, 2020

Commit: 1842e47

@mkoeppe mkoeppe changed the title Replace NumberField_generic.__latex_variable_name by ._latex_name Replace NumberField_generic.__latex_variable_name by ._latex_names Aug 15, 2020
@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 15, 2020

Changed commit from 1842e47 to 6697bca

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 15, 2020

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

6697bcaFixup

@tscrim
Copy link
Collaborator

tscrim commented Aug 15, 2020

Reviewer: Travis Scrimshaw

@tscrim
Copy link
Collaborator

tscrim commented Aug 15, 2020

comment:7

Green patchbot => positive review.

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2020

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

da551dcsrc/sage/rings/number_field/number_field.py: Fix error found by pyflakes

@sagetrac-git
Copy link
Mannequin

sagetrac-git mannequin commented Aug 16, 2020

Changed commit from 6697bca to da551dc

@mkoeppe
Copy link
Contributor Author

mkoeppe commented Aug 16, 2020

comment:9

Thank you! All green

@vbraun
Copy link
Member

vbraun commented Aug 23, 2020

@vbraun vbraun closed this as completed in 027d34e Aug 23, 2020
@sagetrac-fwclarke sagetrac-fwclarke mannequin mentioned this issue Oct 11, 2020
mkoeppe added a commit to mkoeppe/sage that referenced this issue Sep 21, 2023
vbraun pushed a commit to vbraun/sage that referenced this issue Sep 23, 2023
, sagemath#24483, sagemath#24371, sagemath#24511, sagemath#25848, sagemath#26105, sagemath#28481, sagemath#29010, sagemath#29412, sagemath#30332, sagemath#30372, sagemath#31345, sagemath#32375, sagemath#32606, sagemath#32610, sagemath#32612, sagemath#32641, sagemath#32660, sagemath#32750, sagemath#32869, sagemath#33602

    
<!-- ^^^^^
Please provide a concise, informative and self-explanatory title.
Don't put issue numbers in there, do this in the PR body below.
For example, instead of "Fixes sagemath#1234" use "Introduce new method to
calculate 1+1"
-->
<!-- Describe your changes here in detail -->

<!-- Why is this change required? What problem does it solve? -->
<!-- If this PR resolves an open issue, please link to it here. For
example "Fixes sagemath#12345". -->
<!-- If your change requires a documentation PR, please link it
appropriately. -->

### 📝 Checklist

<!-- Put an `x` in all the boxes that apply. -->
<!-- If your change requires a documentation PR, please link it
appropriately -->
<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
<!-- Feel free to remove irrelevant items. -->

- [x] The title is concise, informative, and self-explanatory.
- [ ] The description explains in detail what this PR is about.
- [x] I have linked a relevant issue or discussion.
- [ ] I have created tests covering the changes.
- [ ] I have updated the documentation accordingly.

### ⌛ Dependencies

<!-- List all open PRs that this PR logically depends on
- sagemath#12345: short description why this is a dependency
- sagemath#34567: ...
-->

<!-- If you're unsure about any of these, don't hesitate to ask. We're
here to help! -->
    
URL: sagemath#36307
Reported by: Matthias Köppe
Reviewer(s):
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants