Skip to content

Make PrimitiveGroupsAvailable return false if artifact not downloaded#61

Merged
fingolfin merged 3 commits intogap-packages:masterfrom
lgoettgens:lg/PrimitiveGroupsAvailable
Sep 23, 2025
Merged

Make PrimitiveGroupsAvailable return false if artifact not downloaded#61
fingolfin merged 3 commits intogap-packages:masterfrom
lgoettgens:lg/PrimitiveGroupsAvailable

Conversation

@lgoettgens
Copy link
Copy Markdown
Contributor

This is an attempt to resolve #59.

The check is not optimal as we don't know the nr in PrimitiveGroupsAvailable, but I think it is already good to assume that someone that downloaded at least the first file of the artifact also downloaded the rest of it.

cc @fingolfin

@fingolfin
Copy link
Copy Markdown
Member

Thanks!

Unfortunately CI fails, so this won't make it into GAP 4.15.0-beta2, but it can still get into the final version.

@lgoettgens
Copy link
Copy Markdown
Contributor Author

This now also prints some info message to inform a user about the zenodo artifact (as suggested by @wilfwilson in #59 (comment)).

If I understand the Oscar src code, this info message should be suppressed by https://github.com/oscar-system/Oscar.jl/blob/b16b90b411541c16408c9b3ebd07ebf63b9476e1/src/Oscar.jl#L124, which is intended IMO until oscar-system/Oscar.jl#5293 is implemented.

@codecov
Copy link
Copy Markdown

codecov Bot commented Sep 23, 2025

Codecov Report

❌ Patch coverage is 10.52632% with 17 lines in your changes missing coverage. Please review.
✅ Project coverage is 99.91%. Comparing base (a79d636) to head (abc5c4e).
⚠️ Report is 4 commits behind head on master.

Files with missing lines Patch % Lines
lib/primitiv.gi 10.52% 17 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master      #61      +/-   ##
==========================================
- Coverage   99.91%   99.91%   -0.01%     
==========================================
  Files          46       46              
  Lines      383082   383098      +16     
==========================================
+ Hits       382761   382763       +2     
- Misses        321      335      +14     
Files with missing lines Coverage Δ
lib/primitiv.gi 51.65% <10.52%> (-1.34%) ⬇️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@fingolfin fingolfin mentioned this pull request Sep 23, 2025
7 tasks
Comment thread lib/primitiv.gi
if PrimGrpArtifactFilename(deg,1) <> fail then
return true;
else
Info(InfoWarning,1,"Note that primitive groups of degree 4096 to 8191 must be downloaded separately. They can be obtained from https://doi.org/10.5281/zenodo.10411366");
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This could be really irritating in code that calls this function a lot. Perhaps it should only be shown once?

Anyway, we could also just wait and see if someone complains and then do it then.

Copy link
Copy Markdown
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.

Thanks

@fingolfin fingolfin merged commit 921c3d2 into gap-packages:master Sep 23, 2025
8 of 9 checks passed
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.

PrimitiveGroupsAvailable should only return true for actual available groups

2 participants