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

Some small examples to start with in README.md #245

Merged
merged 1 commit into from
May 21, 2019

Conversation

sebasguts
Copy link
Contributor

No description provided.

@sebasguts sebasguts requested a review from mohamed-barakat May 21, 2019 08:47
@codecov
Copy link

codecov bot commented May 21, 2019

Codecov Report

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

@@           Coverage Diff           @@
##           master     #245   +/-   ##
=======================================
  Coverage   75.38%   75.38%           
=======================================
  Files           5        5           
  Lines         459      459           
=======================================
  Hits          346      346           
  Misses        113      113

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.

Seems fine in principle, but I'd tweak some phrasing. And of course this does not replace a proper manual, where we explain e.g. more about what conversions are possible, and also explain background, e.g. why we don't convert everything automatically.

README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
@fingolfin
Copy link
Member

(Of course I am not asking you to write that manual here -- the change request is purely for some phrasing in you PR).

@sebasguts
Copy link
Contributor Author

I see. I applied your suggests :)

README.md Outdated
julia> x = GAP.julia_to_gap([1,2,3])
GAP: [ 1, 2, 3 ]
```
Converting back to Julia is done using `GAP.gap_to_julia`. However, for this one needs to specify the desired type of the resulting object. For example, to convert the GAP list of integers we just defined back to Julia, we might do this:
Copy link
Member

Choose a reason for hiding this comment

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

Why is this one long line, while lines 29/30 is wrapped?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think because you made this line longer because of your suggestions ;). Anyway, fixed it.

README.md Outdated
```
one may call any GAP function by prefixing its name with `GAP.Globals.`. For example:
```julia
julia> GAP.Globals.SymmetricGroup( 3 )
Copy link
Member

Choose a reason for hiding this comment

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

I am not going to request a change for this, but I note that once more, this is formatted differently in terms of whitespace than the other example inputs :-).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Still fixed it.

@sebasguts sebasguts merged commit 55a72ea into oscar-system:master May 21, 2019
@sebasguts sebasguts deleted the sg/first_release branch May 21, 2019 12:10
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