-
Notifications
You must be signed in to change notification settings - Fork 160
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
Rename RadicalGroup
to SolvableRadical
(the old name is still supported as an obsolete synonym)
#4655
Rename RadicalGroup
to SolvableRadical
(the old name is still supported as an obsolete synonym)
#4655
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there a need to tag the synonym RadicalGroup
immediately as obsolete? I have packages and private code that install methods, and there are publications out that explicitly refer to RadicalGroup
. We can slowly change the princip[al use over, but I don't see a benefit in marking RadicalGroup
for deletion.
@hulpke Would it be sufficient to set the minimal level of Actually, I see no reason to remove obsolete synonyms for names that are regarded as better, except if they are wrong. The situation is different for obsolete code, where one can argue that it would be a burden to maintain it further. |
As long as |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When compiling the reference manual with this PR, there is a warning about an unresolved reference to RadicalGroup
, which therefore needs to be updated to SolvableRadical
. I'm not sure where the broken reference is.
#W WARNING: non resolved reference: rec(
Attr := "RadicalGroup" )
lib/obsolete.gd
Outdated
## </Description> | ||
## </ManSection> | ||
## | ||
## 'RadicalGroup' was documented until version 4.11.1. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should still be documented! Normally when we rename things and make the old name obsolete, then we add an entry for it (including an index entry) to doc/ref/obsolete.xml
, and I think that should be done here.
So I'd rather say:
## 'RadicalGroup' was documented until version 4.11.1. | |
## 'RadicalGroup' was renamed in GAP 4.12 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can make this change.
But my understanding of the word "documented" is a positive one in the sense that one can rely on documented variables, and using undocumented variables can be risky. This was also the idea behind the (undocumented) function IsDocumentedWord
; having index entries for deprecated variable names causes that the current implementation of this function does not fit to its intended meaning.
Concerning the sentence "Normally ...":
Currently we do not claim to have a complete list of name changes in the chapter called "Replaced and Removed Command Names" in the Reference Manual. The section about group actions lists "some examples of such name changes", and the section "Miscellaneous Name Changes or Removed Names" states that it lists "some further name changes". I think it would be better to have a complete list, and to state this --a topic for a new issue.
@wilfwilson Thanks for the hint. This is very strange, in fact many instances of |
and added the treatment of `RadicalGroup` as an obsolescent variable Note that there is the notion of radical (p-)subgroups of a group, which means something completely different, and that solvable radical seems to be the term used in the litarature. (I guess that the name `RadicalGroup` had been chosen --a long time ago-- because `Radical` was too unspecific, and thus adding the suffix `Group` provided at least some context.)
I do not understand how these instances managed to escape from the first commit.
818308f
to
1a09e3c
Compare
1a09e3c
to
74e82b9
Compare
RadicalGroup
to SolvableRadical
RadicalGroup
to SolvableRadical
(the old name is still supported as an obsolete synonym)
and added the treatment of
RadicalGroup
as an obsolescent variableNote that there is the notion of radical (p-)subgroups of a group,
which means something completely different,
and that solvable radical seems to be the term used in the litarature.
(I guess that the name
RadicalGroup
had been chosen --a long time ago--because
Radical
was too unspecific, and thus adding the suffixGroup
provided at least some context.)
Please provide a short summary of this PR and its purpose here. E.g., does it add a new feature, and which? Does it fix a bug, and which? If there is an associated issue, please list it here.
Text for release notes
We track noteworthy changes as entries in the
CHANGES.md
file in the root directory of this repository. To help us decide whether and how to describe your PR, please do one of the following:CHANGES.md
, and write "see title" here (or if you have the required permissions, add the labelrelease notes: use title
to this PR and remove this section)Further details
If necessary, provide further details down here.