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

Switch to explicit imports of names from packages #748

Merged
merged 8 commits into from
Mar 4, 2024
Merged

Conversation

dmbates
Copy link
Collaborator

@dmbates dmbates commented Mar 1, 2024

With the availability of ExplicitImports.jl it is feasible to list all the symbols used from a package in the dependencies instead of importing all the exported symbols.

These changes are based on the output of ExplicitImports.print_explicit_imports(MixedModels).

Did behavior change? No, the behavior didn't (or shouldn't have) change

  • add entry in NEWS.md
  • after opening this PR, add a reference and run docs/NEWS-update.jl to update the cross-references.

Should we release your changes right away? If so, bump the version:

  • I've bumped the version appropriately

@dmbates dmbates requested a review from palday March 1, 2024 19:22
Copy link

codecov bot commented Mar 1, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 96.33%. Comparing base (529c710) to head (d4ff897).

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #748   +/-   ##
=======================================
  Coverage   96.33%   96.33%           
=======================================
  Files          34       34           
  Lines        3353     3353           
=======================================
  Hits         3230     3230           
  Misses        123      123           
Flag Coverage Δ
current 96.27% <ø> (-0.06%) ⬇️
minimum 96.23% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Contributor

github-actions bot commented Mar 1, 2024

Benchmark Report for /home/runner/work/MixedModels.jl/MixedModels.jl

Job Properties

  • Time of benchmarks:
    • Target: 4 Mar 2024 - 15:20
    • Baseline: 4 Mar 2024 - 15:24
  • Package commits:
    • Target: 214a4c
    • Baseline: 529c71
  • Julia commits:
    • Target: bd47ec
    • Baseline: bd47ec
  • Julia command flags:
    • Target: None
    • Baseline: -C,native,-J/opt/hostedtoolcache/julia/1.10.2/x64/lib/julia/sys.so,-g1,-O3,-e,using Pkg; Pkg.update(); Pkg.add(["BenchmarkTools", "StatsModels"])
  • Environment variables:
    • Target: None
    • Baseline: None

Results

A ratio greater than 1.0 denotes a possible regression (marked with ❌), while a ratio less
than 1.0 denotes a possible improvement (marked with ✅). Only significant results - results
that indicate possible regressions or improvements - are shown below (thus, an empty table means that all
benchmark results remained invariant between builds).

ID time ratio memory ratio
["crossed", "mrk17_exp1:1"] 0.08 (5%) ✅ 1.00 (1%)
["crossedvector", "d3:1"] 0.94 (5%) ✅ 1.00 (1%)
["crossedvector", "kb07:3"] 0.45 (5%) ✅ 1.00 (1%)
["nested", "pastes:2"] 0.94 (5%) ✅ 1.00 (1%)

Benchmark Group List

Here's a list of all the benchmark groups executed by this job:

  • ["crossed"]
  • ["crossedvector"]
  • ["nested"]
  • ["singlevector"]

Julia versioninfo

Target

Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
      Ubuntu 22.04.4 LTS
  uname: Linux 6.5.0-1015-azure #15~22.04.1-Ubuntu SMP Tue Feb 13 01:15:12 UTC 2024 x86_64 x86_64
  CPU: AMD EPYC 7763 64-Core Processor: 
              speed         user         nice          sys         idle          irq
       #1  2445 MHz        963 s          0 s        262 s       3026 s          0 s
       #2  2597 MHz       1083 s          0 s        404 s       2768 s          0 s
       #3  3242 MHz       1266 s          0 s        255 s       2725 s          0 s
       #4  2995 MHz       1667 s          0 s        291 s       2283 s          0 s
  Memory: 15.606491088867188 GB (13811.50390625 MB free)
  Uptime: 428.47 sec
  Load Avg:  1.73  1.19  0.56
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, znver3)
Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores)

Baseline

Julia Version 1.10.2
Commit bd47eca2c8a (2024-03-01 10:14 UTC)
Build Info:
  Official https://julialang.org/ release
Platform Info:
  OS: Linux (x86_64-linux-gnu)
      Ubuntu 22.04.4 LTS
  uname: Linux 6.5.0-1015-azure #15~22.04.1-Ubuntu SMP Tue Feb 13 01:15:12 UTC 2024 x86_64 x86_64
  CPU: AMD EPYC 7763 64-Core Processor: 
              speed         user         nice          sys         idle          irq
       #1  3243 MHz       1289 s          0 s        377 s       4642 s          0 s
       #2  3255 MHz       1635 s          0 s        597 s       4082 s          0 s
       #3  2445 MHz       1766 s          0 s        331 s       4208 s          0 s
       #4  3194 MHz       2767 s          0 s        426 s       3109 s          0 s
  Memory: 15.606491088867188 GB (13887.4921875 MB free)
  Uptime: 634.87 sec
  Load Avg:  1.6  1.34  0.75
  WORD_SIZE: 64
  LIBM: libopenlibm
  LLVM: libLLVM-15.0.7 (ORCJIT, znver3)
Threads: 1 default, 0 interactive, 1 GC (on 4 virtual cores)

@dmbates
Copy link
Collaborator Author

dmbates commented Mar 1, 2024

It appears that the test failure on nightly is the old problem of not finding the datasets to download. I'm not sure why the other Julia releases can do this but the nightly release fails to do so.

@ericphanson
Copy link

btw do file an issue if you run into any issues! Also if you wish, you can add ExplicitExports as a test dep and use the check_* functions to ensure no implicit exports or stale explicit exports.

NEWS.md Outdated Show resolved Hide resolved
Project.toml Outdated
@@ -20,6 +20,7 @@ ProgressMeter = "92933f4c-e287-5a05-a399-4b506db050ca"
Random = "9a3f8284-a2c9-5f02-9a11-845980a1fd5c"
SparseArrays = "2f01184e-e22b-5df5-ae63-d93ebab69eaf"
StaticArrays = "90137ffa-7385-5640-81b9-e52037218182"
StaticArraysCore = "1e83bf80-4336-4d27-bf5d-d5a4f845583c"
Copy link
Member

Choose a reason for hiding this comment

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

since we already have StaticArrays, can we grab SVector from there? I know this doesn't increase our total dependency load, but it's another thing to track.

Choose a reason for hiding this comment

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

filed ericphanson/ExplicitImports.jl#24 fyi. I think though if you do using StaticArrays: SVector, then it won't complain about the implicit import. But when it makes suggestions, it always does the "defining" module.

src/MixedModels.jl Outdated Show resolved Hide resolved
@dmbates dmbates merged commit ee5f2cf into main Mar 4, 2024
11 of 12 checks passed
@dmbates dmbates deleted the db/explicitimports branch March 4, 2024 15:37
@ericphanson
Copy link

BTW I put a PR to improve the printing, so next time you wouldn't have to manually regroup as much: ericphanson/ExplicitImports.jl#27

@dmbates
Copy link
Collaborator Author

dmbates commented Mar 5, 2024

Thank you, @ericphanson , for implementing this change in the printing. That's very helpful.

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.

3 participants