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

Add CI tests to our projects looking for "bad" exports #1964

Closed
2 tasks
fingolfin opened this issue Feb 22, 2023 · 5 comments · Fixed by #2370
Closed
2 tasks

Add CI tests to our projects looking for "bad" exports #1964

fingolfin opened this issue Feb 22, 2023 · 5 comments · Fixed by #2370
Assignees

Comments

@fingolfin
Copy link
Member

We sometimes accidentally export non-existing things. E.g. from current Oscar.jl:

julia> filter(n->!isdefined(Oscar, n), names(Oscar))
32-element Vector{Symbol}:
 :AbsPrePreSheaf
 :CoherentSheaf
 :DirectProductOfElem
 :LineBundle
 :_non_degeneration_cover
 :affine_patch_type
 :ambient
 :as_smooth_lci_of_cod
 :as_smooth_local_complete_intersection
 :base_scheme_type
 :controlled_transform
 :coset_decomposition
 :covered_patches
 :covering
 :dualsubdivision
 :generator
 :has_complement_class_reps
 :ideal_dict
 :ideal_sheaf_type
 :images_of_variables
 :intersect_stably
 :load_simplicialcomplex
 :normalize
 :p
 :prepare_smooth_center
 :save_simplicialcomplex
 :set_complement_class_reps
 :set_is_finite
 :subdivision_of_vertices
 :total_transform
 :weak_transform
 :weight

The :p one is due to a typo, @HereAround is already fixing it.

I suggest the following:

  • resolve these (either by adding missing functions or removing the exports)
  • add something like this to our tests:
    @test all(n->isdefined(Oscar, n), names(Oscar))
  • add similar tests to our other packages (AA, Nemo, Hecke, GAP, Singular, Polymake, ...)
@HereAround
Copy link
Member

For completeness, the fix for export p is included in the PR #1958.

@thofma
Copy link
Collaborator

thofma commented Feb 22, 2023

We could just use https://github.com/JuliaTesting/Aqua.jl (I used it successfully locally in the past).

@afkafkafk13
Copy link
Collaborator

I am working on
:total_transform
:weak_transform
:controlled_transform
right now.

@lkastner
Copy link
Member

lkastner commented May 7, 2023

I like @thofma s suggestion very much.

lkastner added a commit that referenced this issue May 9, 2023
lkastner added a commit that referenced this issue May 9, 2023
lkastner added a commit that referenced this issue May 9, 2023
@lgoettgens
Copy link
Member

We could just use https://github.com/JuliaTesting/Aqua.jl (I used it successfully locally in the past).

I added a first try to use it in #2370 now that the wrong exports have been removed (thanks, @lkastner!).

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 a pull request may close this issue.

6 participants