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

document basic representations of objects (IsInternalRep, IsDataObjectRep, IsComponentObjectRep, IsPositionalObjectRep, IsAttributeStoringRep, IsPlistRep) #3615

Merged

Conversation

ThomasBreuer
Copy link
Contributor

  • documented IsInternalRep, IsDataObjectRep, IsComponentObjectRep,
    IsPositionalObjectRep, IsAttributeStoringRep, IsPlistRep

    Up to now, these representations were either not documented at all
    or just via <Index> entries in some general text.
    I would like to refer to some of these representations in the documentation for IsMatrixObj and IsVectorObj; for that, they should be properly documented.

  • adjusted the relevant cross-references:
    Use <Ref...> instead of <C> elements.

@ThomasBreuer ThomasBreuer added topic: documentation Issues and PRs related to documentation release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes gapsingular2019 Issues and PRs that arose at https://opendreamkit.org/meetings/2019-04-02-GAPSingularMeeting labels Aug 21, 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.

Overall a nice improvement, thanks! Just two minor remarks.

lib/list.gd Outdated
##
## <Description>
## &GAP; lists created by entering comma separated values in square brackets
## are represented internally as so-called <E>plain lists</E>,
Copy link
Member

Choose a reason for hiding this comment

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

Note that this is not 100% correct: for boolean lists, GAP returns a blist rep:

gap> IsPlistRep( [ true, false ] );
false
gap> IsBlistRep( [ true, false ] );
true

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point. I will fix this.

lib/type.g Outdated
## <!-- This will be changed eventually, in order to avoid conflicts between-->
## <!-- ordinary components and components corresponding to attributes.-->
## This will be changed eventually, in order to avoid conflicts between
## ordinary components and components corresponding to attributes.-->
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should just remove those last 4-5 lines?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes.
Note that the rule for the component names had not been commented out before, but still it was not part of the documentation since the GAPDoc "chunk" was not requested by any of the XML files.

- document `IsInternalRep`, `IsDataObjectRep`, `IsComponentObjectRep`,
  `IsPositionalObjectRep`, `IsAttributeStoringRep`, `IsPlistRep`

  Up to now, these representations were either not documented at all
  or just via `<Index>` entries in some general text.

- adjusted the relevant cross-references:
  Use `<Ref...>` instead of `<C>` elements.
@codecov
Copy link

codecov bot commented Aug 22, 2019

Codecov Report

Merging #3615 into master will decrease coverage by 12.37%.
The diff coverage is n/a.

@@             Coverage Diff             @@
##           master    #3615       +/-   ##
===========================================
- Coverage   84.64%   72.27%   -12.38%     
===========================================
  Files         698      635       -63     
  Lines      345549   308791    -36758     
===========================================
- Hits       292483   223171    -69312     
- Misses      53066    85620    +32554
Impacted Files Coverage Δ
lib/list.gd 100% <ø> (ø) ⬆️
lib/type.g 77.45% <ø> (-5.4%) ⬇️
lib/string.g 97.91% <ø> (-2.09%) ⬇️
lib/object.gd 93.02% <ø> (-4.66%) ⬇️
lib/teachm2.g 15.38% <0%> (-84.62%) ⬇️
lib/ctblauto.gi 5.98% <0%> (-83.67%) ⬇️
lib/helpt2t.gi 0.23% <0%> (-83.14%) ⬇️
lib/proto.gi 1.03% <0%> (-82.48%) ⬇️
src/compiler.c 8% <0%> (-80.68%) ⬇️
lib/ctbllatt.gi 0.81% <0%> (-79.99%) ⬇️
... and 385 more

@ThomasBreuer
Copy link
Contributor Author

@fingolfin I have updated my pull request according to your comments.

@ThomasBreuer ThomasBreuer merged commit 1c15c63 into gap-system:master Aug 22, 2019
@fingolfin fingolfin changed the title document basic representations of objects document basic representations of objects (IsInternalRep, IsDataObjectRep, IsComponentObjectRep, IsPositionalObjectRep, IsAttributeStoringRep, IsPlistRep) Aug 22, 2019
@fingolfin fingolfin added release notes: added PRs introducing changes that have since been mentioned in the release notes and removed release notes: to be added PRs introducing changes that should be (but have not yet been) mentioned in the release notes labels Aug 22, 2019
@coveralls
Copy link

coveralls commented Aug 22, 2019

Coverage Status

Coverage decreased (-0.0003%) to 84.395% when pulling 93668e2 on ThomasBreuer:TB_document_representations into 65454fe on gap-system:master.

@olexandr-konovalov olexandr-konovalov added this to the GAP 4.11.0 milestone Feb 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
gapsingular2019 Issues and PRs that arose at https://opendreamkit.org/meetings/2019-04-02-GAPSingularMeeting release notes: added PRs introducing changes that have since been mentioned in the release notes topic: documentation Issues and PRs related to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants