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 it possible to generate GAP functions from Julia #212

Merged

Conversation

sebasguts
Copy link
Contributor

No description provided.

@sebasguts sebasguts requested a review from fingolfin March 9, 2019 14:57
@sebasguts
Copy link
Contributor Author

This allows for:

               _
   _       _ _(_)_     |  Documentation: https://docs.julialang.org
  (_)     | (_) (_)    |
   _ _   _| |_  __ _   |  Type "?" for help, "]?" for Pkg help.
  | | | | | | |/ _` |  |
  | | |_| | | | (_| |  |  Version 1.1.0 (2019-01-21)
 _/ |\__'_|_|_|\__'_|  |  Official https://julialang.org/ release
|__/                   |

julia> using GAP
Adding path /home/sebastian/Software/GAPJulia/gap/.libs to DL_LOAD_PATH
 ┌───────┐   GAP 4.dev of today
 │  GAP  │   https://www.gap-system.org
 └───────┘   Architecture: x86_64-pc-linux-gnu-julia64-kv6
 Configuration:  gmp 6.1.2, Julia GC, Julia 1.1.0, readline
 Loading the library and packages ...
^[[B Packages:   GAPDoc 1.6.1, PrimGrp 3.3.1, SmallGrp 1.3, TransGrp 2.0.4
 Try '??help' for help. See also '?copyright', '?cite' and '?authors'

julia> oper = (i,g)->i^g
#3 (generic function with 1 method)

julia> GAP.Globals.Orbit(GAP.Globals.SymmetricGroup(3),1,GAP.julia_to_gap(oper))
GAP: [ 1, 2, 3 ]

@codecov
Copy link

codecov bot commented Mar 9, 2019

Codecov Report

Merging #212 into master will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #212   +/-   ##
=======================================
  Coverage   91.31%   91.31%           
=======================================
  Files          25       25           
  Lines        1486     1486           
=======================================
  Hits         1357     1357           
  Misses        129      129

@fingolfin
Copy link
Member

Please rebase so that CI runs again and we get proper coverage.

@sebasguts sebasguts force-pushed the generate_gap_funcs_from_julia branch from f60a118 to 97fa9a7 Compare March 10, 2019 13:48
@sebasguts
Copy link
Contributor Author

Done.

@fingolfin
Copy link
Member

Clearly something is still broken wrt coverage, the C code has no coverzreported (and hence this PR shows no coverage).

@fingolfin
Copy link
Member

Am on a phone only now, but: do we still oasd --coverage in CFLAGS and LDFLAGS when compiling our code? (Possibly indirectly, if GAP was compiled with those flags)

@sebasguts
Copy link
Contributor Author

@fingolfin We don't for the GAP compiled by julia, I assume. Should I add this simply to the debug mode? Guess it won't hurt there.

fingolfin
fingolfin previously approved these changes Mar 11, 2019
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.

While we don't get proper coverage return now, it is clear there there is a test case here, so all should be fine (and if not, we can revisit it later).
I'd perhaps insert one space after a comma, but other than that, let's just merge this.

test/conversion.jl Outdated Show resolved Hide resolved
@sebasguts sebasguts merged commit 60fbdb1 into oscar-system:master Mar 11, 2019
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