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

Make abelian closure respect unicode setting #2437

Merged
merged 1 commit into from Jun 1, 2023
Merged

Conversation

thofma
Copy link
Collaborator

@thofma thofma commented Jun 1, 2023

No description provided.

@thofma thofma requested a review from ThomasBreuer June 1, 2023 07:01
Copy link
Member

@ThomasBreuer ThomasBreuer left a comment

Choose a reason for hiding this comment

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

Looks good, thanks.
Thus the "fancy" output shall be avoided not only in the code but also in docstrings.

return K.s
else
if Oscar.is_unicode_allowed()
return "ζ"
Copy link
Member

Choose a reason for hiding this comment

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

If we want to avoid the explicit "ζ" in the code then we can also write transcode(String, [UInt16(950)]).
(The 950 can be computed as Int(codepoint('ζ')).)

Copy link
Member

Choose a reason for hiding this comment

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

I think this should be fine as-is. At least we have similar things in other places.

@codecov
Copy link

codecov bot commented Jun 1, 2023

Codecov Report

Merging #2437 (e41ca36) into master (d92e9e8) will increase coverage by 0.00%.
The diff coverage is 66.66%.

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #2437   +/-   ##
=======================================
  Coverage   71.61%   71.62%           
=======================================
  Files         393      393           
  Lines       52863    52868    +5     
=======================================
+ Hits        37858    37866    +8     
+ Misses      15005    15002    -3     
Impacted Files Coverage Δ
src/Rings/binomial_ideals.jl 88.49% <ø> (ø)
src/Rings/AbelianClosure.jl 88.28% <66.66%> (+0.12%) ⬆️

... and 2 files with indirect coverage changes

@fingolfin fingolfin merged commit 7c212d4 into master Jun 1, 2023
15 checks passed
@fingolfin fingolfin deleted the th/abelianclosure branch June 1, 2023 09:43
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.

None yet

3 participants