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

Clean up for abelian closure #571

Merged
merged 1 commit into from
Jul 23, 2021
Merged

Clean up for abelian closure #571

merged 1 commit into from
Jul 23, 2021

Conversation

thofma
Copy link
Collaborator

@thofma thofma commented Jul 19, 2021

Now with tests and documentation.

@thofma
Copy link
Collaborator Author

thofma commented Jul 19, 2021

Should wait for #570.

@thofma thofma force-pushed the th/qab branch 3 times, most recently from 86c124e to c86fd74 Compare July 20, 2021 07:02
src/Rings/AbelianClosure.jl Outdated Show resolved Hide resolved
@tthsqe12
Copy link
Contributor

Looks like you changed the printing of z_n to z(n).


_variable(K::QabField) = K.s

_variable(b::QabElem) = string(_variable(_Qab), "(", b.c, ")")
Copy link
Contributor

@tthsqe12 tthsqe12 Jul 21, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For expressification purposes, it would be best if this was
Expr(:call, Symbol(_variable(_Qab)), b.c)

Plain strings are the worst.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Will do. Will also use the name macro.

@thofma thofma merged commit 9d7dc0f into master Jul 23, 2021
@fingolfin fingolfin deleted the th/qab branch July 23, 2021 13:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants