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

do not mention fmpz in the JuliaInterface manual #857

Merged
merged 4 commits into from
Feb 23, 2023

Conversation

ThomasBreuer
Copy link
Member

No description provided.

@codecov
Copy link

codecov bot commented Feb 22, 2023

Codecov Report

Merging #857 (5478bbd) into master (5af420d) will decrease coverage by 6.30%.
The diff coverage is n/a.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #857      +/-   ##
==========================================
- Coverage   81.78%   75.49%   -6.30%     
==========================================
  Files          49       51       +2     
  Lines        3679     4101     +422     
==========================================
+ Hits         3009     3096      +87     
- Misses        670     1005     +335     
Impacted Files Coverage Δ
pkg/JuliaInterface/gap/JuliaInterface.gd 100.00% <ø> (ø)
pkg/JuliaInterface/gap/calls.gi 100.00% <ø> (ø)
src/utils.jl 38.09% <ø> (ø)
pkg/JuliaExperimental/gap/finvar.gd 100.00% <0.00%> (ø)
pkg/JuliaExperimental/gap/finvar.gi 14.68% <0.00%> (ø)
pkg/JuliaExperimental/read.g 100.00% <0.00%> (+7.69%) ⬆️

#! is handled in the context of the <Package>Oscar</Package> system,
#! which uses both &GAP; and <Package>Nemo.jl</Package>,
#! but <Package>JuliaInterface</Package> does not deal with it.
#! For example, the &Julia; package <Package>Oscar.jl</Package> defines
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
#! For example, the &Julia; package <Package>Oscar.jl</Package> defines
#! For example, the &Julia; package <Package>Oscar.jl</Package> provides

Then it also encompasses fmpz

Copy link
Member

@fingolfin fingolfin left a comment

Choose a reason for hiding this comment

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

OK by me. I'd consider tweaking some details, we can talk about it tomorrow (or you just merge, either with or without my suggestions)

@@ -5,7 +5,7 @@ BindGlobal("_JL_Dict_Any", JuliaEvalString("Dict{Symbol,Any}"));

##
## We want to use &GAP's function call syntax also for certain Julia objects
## that are <E>not</E> functions, for example for types such as <C>fmpz</C>.
## that are <E>not</E> functions, for example <C>ZZ</C>.
Copy link
Member

Choose a reason for hiding this comment

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

or we could just reference a base type?

Suggested change
## that are <E>not</E> functions, for example <C>ZZ</C>.
## that are <E>not</E> functions, for example for types such as <C>String</C>.

(true, 1.4142135623730951)

julia> GAP.call_with_catch( sqrt, -2 )
(false, "DomainError(-2.0, \\"sqrt will only return a complex result if called with a complex argument. Try sqrt(Complex(x)).\\")")
Copy link
Member

Choose a reason for hiding this comment

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

It seems there is a similar problem (changes in the error in Julia nightly) inside pkg/JuliaInterface/tst/utils.tst

@fingolfin fingolfin merged commit d7c4dca into oscar-system:master Feb 23, 2023
@ThomasBreuer ThomasBreuer deleted the TB_no_fmpz branch February 23, 2023 10:09
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