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

renaming PreImagesRepresentative as NC version #31

Merged
merged 1 commit into from
Feb 13, 2023
Merged

renaming PreImagesRepresentative as NC version #31

merged 1 commit into from
Feb 13, 2023

Conversation

cdwensley
Copy link
Contributor

PreImagesRepresetnative, PreImages, PreImagesSet and PreImagesElm can all return incorrect results when the element(s) supplied are not in the range of the map.
This situation has been discussed in GAP issue #4809.
To rectify the situation the plan is to have NC versions of these four operations and to add tests to the non-NC versions.
The procedure to be adopted is as follows.
(1) Rename the four operations by adding 'NC' to their names, and then declare the original operations as synonyms of these. PR #5073 addresses this.
(2) Ask package authors/maintainers to convert all their calls to these functions to the NC versions (unless there is good reason not to do so).
A clause added to init.g ensures the package works in older versions of GAP.
(3) Once all the packages have been updated, add tests to the non-NC versions of the operations.
This PR attempts to implement (2) for package cryst which only uses the first of these 4 operations.

@codecov
Copy link

codecov bot commented Oct 6, 2022

Codecov Report

Merging #31 (4df008f) into master (f1a0f4f) will not change coverage.
The diff coverage is 88.88%.

@@           Coverage Diff           @@
##           master      #31   +/-   ##
=======================================
  Coverage   94.32%   94.32%           
=======================================
  Files          24       24           
  Lines        7573     7573           
=======================================
  Hits         7143     7143           
  Misses        430      430           
Impacted Files Coverage Δ
gap/pcpgrp.gi 89.51% <60.00%> (ø)
gap/orbstab.gi 86.56% <83.33%> (ø)
gap/cryst.gi 85.62% <100.00%> (ø)
gap/cryst2.gi 86.21% <100.00%> (ø)
gap/equiv.gi 81.41% <100.00%> (ø)
gap/fpgrp.gi 73.10% <100.00%> (ø)
gap/hom.gi 82.14% <100.00%> (ø)
gap/zass.gi 86.88% <100.00%> (ø)

@gaehler gaehler merged commit 3a0d8ae into gap-packages:master Feb 13, 2023
@cdwensley
Copy link
Contributor Author

Thanks for merging this. Nothing has happened with GAP PR #5073 since October, but your commit helps to encourage me to get back to this development. It will take a long time.

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