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.

Sign up for GitHub

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

Jump to bottom

add docstrings for acting_group and action_function #3432

Merged
merged 6 commits into from
Feb 27, 2024
Merged

add docstrings for acting_group and action_function #3432

merged 6 commits into from
Feb 27, 2024

Conversation

mjrodgers
Copy link
Collaborator

@mjrodgers mjrodgers commented Feb 26, 2024 •
edited
Loading

Closes #3370

lgoettgens
lgoettgens reviewed Feb 26, 2024
View reviewed changes
src/Groups/gsets.jl Outdated Show resolved Hide resolved
@lgoettgens lgoettgens added documentation Improvements or additions to documentation topic: groups labels Feb 26, 2024
ThomasBreuer
ThomasBreuer requested changes Feb 26, 2024
View reviewed changes
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.

Thanks. Just a few useful additions:

  • The documentation for the two functions should be mentioned in docs/src/Groups/action.md.
  • action_function should be added to src/exports.jl.
  • Some doctests could be added.

src/Groups/gsets.jl Show resolved Hide resolved
src/Groups/gsets.jl Show resolved Hide resolved
@fingolfin
Copy link
Member

Thanks that's a good start, but we should also verify that those functions are actually exported - I don't think they are (at least not both).

Perhaps you could add a simple jldoctest to both docstrings showing these functions, I think this then also automatically checks that they are exported.

@fingolfin
Copy link
Member

It seems @ThomasBreuer and me had the same thoughts at about the same moment, so our comments crossed :-)

@lgoettgens
Copy link
Member

The documentation for the two functions should be mentioned in docs/src/Groups/action.md.

This request of @ThomasBreuer is still open

@mjrodgers
Copy link
Collaborator Author

This request of @ThomasBreuer is still open

Thanks, I overlooked this one - I will update now

ThomasBreuer
ThomasBreuer approved these changes Feb 27, 2024
View reviewed changes
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.

Thanks, looks good.

@mjrodgers
Copy link
Collaborator Author

Okay, this should have everything addressed now. I also mentioned using collect(Omega) to obtain the underlying G-set in action.md, so this should suffice to address everything now from #3370

@fingolfin fingolfin merged commit c58c7eb into oscar-system:master Feb 27, 2024
23 checks passed
@lgoettgens
Copy link
Member

Backport? Most of this is just documentation enhancements

mjrodgers added a commit to mjrodgers/Oscar.jl that referenced this pull request Feb 28, 2024
@mjrodgers
add docstrings for acting_group and action_function (oscar-system…
fe2857d 
…#3432)
@benlorenz
Copy link
Member

The new exports are used in the groups chapter, adding it to the backports list.

@benlorenz benlorenz added the backport 1.0.x Should be backported to the release 1.0 branch label Feb 29, 2024
@benlorenz benlorenz mentioned this pull request Feb 29, 2024
Merged
31 tasks
aaruni96 pushed a commit that referenced this pull request Feb 29, 2024
@mjrodgers @aaruni96
add docstrings for acting_group and action_function (#3432)
27876bb 
(cherry picked from commit c58c7eb)
@aaruni96 aaruni96 removed the backport 1.0.x Should be backported to the release 1.0 branch label Feb 29, 2024
benlorenz added a commit that referenced this pull request Feb 29, 2024
@benlorenz
Backports for Oscar 1.0
0eb303b 
- Add QQBar docs to the manual #3423
- do not show the OscarInterface banner #3422
- fix bugs in all_OD_infos #3419
- Ep/ Rename Spec to AffineScheme #3345 #3425
- Remove two mentions of Arb_jll #3431
- Tweak epimorphism_from_free_group #3430
- CI: re-enable nightly #3435
- support gen(G::GAPGroup, 0) #3332
- Align all_*_groups methods some more #3433
- Add all_perfect_groups #3434
- Add all_primitive_groups and all_transitive_groups variants taking a single int or int range #3404
- fix a docstring #3436
- Fixes multivariate division #3396
- Docu invariants tori #3428
- Improve docstrings for is_conjugate/is_conjugate_with_data. #3384
- Fix ambient_module(M::SubquoModule) #3448
- Bugfix for printing of affine schemes #3437
- Bugfix for bugfix for printing of affine schemes #3445
- Update OSCAR banner #3410
- Docu invariants lin. red. groups (Lakshmi Ramesh and Wolfram Decker) #3443
- add od_from_atlas_group, od_from_p_subgroup, and helpers #3444
- Unexport normalise #3453
- support group properties for character tables #3449
- add docstrings for acting_group and action_function #3432 (exports are used in new groups code for the book)
- Adjust to renaming of rank(A::FinGenAbGroup) to torsion_free_rank(A::FinGenAbGroup) #3457
- Ensure fp_group(G) transfers group attributes #3464
- Added comment on convention #3467
- Export weierstrass_chart_on_minimal_model and patch transform_to_weierstrass #3458
- Fix a doc signature #3466
- Grading + caching for affine algebra of torus invariants #3469
@mjrodgers mjrodgers deleted the gset_method_docs branch March 1, 2024 13:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lgoettgens lgoettgens lgoettgens left review comments

@ThomasBreuer ThomasBreuer ThomasBreuer approved these changes

@fingolfin fingolfin fingolfin approved these changes

Assignees
No one assigned
Labels
documentation Improvements or additions to documentation topic: groups
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

G-sets: define and document an interface for getting group,domain,action
6 participants
@mjrodgers @fingolfin @lgoettgens @benlorenz @ThomasBreuer @aaruni96