-
Notifications
You must be signed in to change notification settings - Fork 2
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 aqua, remove pirated methods #96
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #96 +/- ##
==========================================
+ Coverage 53.32% 56.45% +3.13%
==========================================
Files 7 7
Lines 557 542 -15
==========================================
+ Hits 297 306 +9
+ Misses 260 236 -24 ☔ View full report in Codecov by Sentry. |
8c7e5d4
to
608b939
Compare
This comment was marked as resolved.
This comment was marked as resolved.
9bd7afb
to
6fe6d74
Compare
7789560
to
7bfad69
Compare
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.
Looks good. I like the examples that you added in the docstrings.
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.
Will address tomorrow
44146c3
to
f0b1796
Compare
Just noticed we can also drop This brings the Before
julia> @time_imports using HerbGrammar
5.8 ms MacroTools
0.6 ms SimpleTraits
5.2 ms Preferences
0.4 ms PrecompileTools
0.6 ms StaticArraysCore
101.5 ms StaticArrays
1.1 ms ArnoldiMethod
0.7 ms Statistics
0.3 ms StaticArrays → StaticArraysStatisticsExt
0.5 ms Compat
0.5 ms Inflate
2.0 ms OrderedCollections
12.8 ms DataStructures
0.4 ms Compat → CompatLinearAlgebraExt
┌ 9.7 ms SuiteSparse_jll.__init__()
11.7 ms SuiteSparse_jll
0.9 ms Serialization
┌ 6.6 ms SparseArrays.CHOLMOD.__init__() 57.96% compilation time
116.0 ms SparseArrays 3.27% compilation time
0.5 ms Statistics → SparseArraysExt
┌ 0.0 ms Distributed.__init__()
16.6 ms Distributed 53.83% compilation time
0.7 ms Mmap
2.0 ms SharedArrays
8.2 ms Graphs
0.6 ms LaTeXStrings
0.6 ms JLLWrappers
┌ 2.8 ms Bzip2_jll.__init__() 54.90% compilation time
3.4 ms Bzip2_jll 44.12% compilation time
┌ 1.3 ms Zlib_jll.__init__()
3.4 ms Zlib_jll
┌ 1.5 ms FreeType2_jll.__init__()
2.1 ms FreeType2_jll
0.4 ms Libuuid_jll
┌ 0.9 ms Expat_jll.__init__()
1.5 ms Expat_jll
┌ 7.9 ms Fontconfig_jll.__init__() 80.06% compilation time
8.6 ms Fontconfig_jll 72.95% compilation time
┌ 1.1 ms Graphite2_jll.__init__()
1.8 ms Graphite2_jll
┌ 2.0 ms Libiconv_jll.__init__()
2.6 ms Libiconv_jll
┌ 1.1 ms Libffi_jll.__init__()
1.7 ms Libffi_jll
┌ 11.6 ms CompilerSupportLibraries_jll.__init__()
14.4 ms CompilerSupportLibraries_jll
┌ 1.6 ms XML2_jll.__init__()
2.3 ms XML2_jll
┌ 2.4 ms Gettext_jll.__init__()
3.1 ms Gettext_jll
┌ 1.3 ms PCRE2_jll.__init__()
3.1 ms PCRE2_jll
┌ 10.3 ms Glib_jll.__init__()
11.0 ms Glib_jll
┌ 2.0 ms LLVMOpenMP_jll.__init__()
2.7 ms LLVMOpenMP_jll
┌ 2.0 ms Pixman_jll.__init__()
2.6 ms Pixman_jll
┌ 1.1 ms libpng_jll.__init__()
1.7 ms libpng_jll
┌ 3.4 ms Cairo_jll.__init__()
4.1 ms Cairo_jll
┌ 3.6 ms HarfBuzz_jll.__init__()
4.3 ms HarfBuzz_jll
┌ 6.2 ms ICU_jll.__init__()
7.0 ms ICU_jll
┌ 1.1 ms HarfBuzz_ICU_jll.__init__()
1.9 ms HarfBuzz_ICU_jll
┌ 4.4 ms OpenSSL_jll.__init__()
5.2 ms OpenSSL_jll
┌ 0.0 ms tectonic_jll.__init__()
0.8 ms tectonic_jll
┌ 0.0 ms Requires.__init__()
0.7 ms Requires
┌ 2.7 ms JpegTurbo_jll.__init__()
3.5 ms JpegTurbo_jll
┌ 1.2 ms LERC_jll.__init__()
2.0 ms LERC_jll
┌ 1.3 ms Zstd_jll.__init__()
2.1 ms Zstd_jll
┌ 1.5 ms Libtiff_jll.__init__()
2.2 ms Libtiff_jll
┌ 1.5 ms LittleCMS_jll.__init__()
2.3 ms LittleCMS_jll
┌ 1.5 ms OpenJpeg_jll.__init__()
2.4 ms OpenJpeg_jll
┌ 6.3 ms Poppler_jll.__init__()
7.3 ms Poppler_jll
┌ 63.2 ms TikzPictures.__init__() 42.66% compilation time
67.6 ms TikzPictures 39.89% compilation time
1.0 ms TikzGraphs
0.8 ms CommonSubexpressions
0.8 ms TreeView
5.4 ms AbstractTrees
1.1 ms HerbCore
928.5 ms HerbGrammar After
julia> @time_imports using HerbGrammar
2.9 ms Serialization
2.1 ms HerbCore
924.6 ms HerbGrammar and brings the number of transitive dependencies down by a lot: Beforejulia> length(Pkg.dependencies())
104 Afterjulia> length(Pkg.dependencies())
3 |
588e13a
to
91b51ad
Compare
They don't seem to be used at the point in time, and their utility seems fairly vague anyway.
91b51ad
to
6bcebbc
Compare
These were removed from `HerbGrammar` in version v0.5. See: Herb-AI/HerbGrammar.jl#96
These were removed from `HerbGrammar` in version v0.5. See: Herb-AI/HerbGrammar.jl#96
These methods were picked up by Aqua as being pirated. They should be defined in
HerbCore
. See Herb-AI/HerbCore.jl#31. Removing these methods from this package means this is also a breaking change.Aqua tests are now included in CI so that this won't happen by accident in the future.
The reason for changing the
RuleNode(..., grammar)
andUniformHole(..., grammar)
constructors to functions was also for type-piracy reasons--defining a new constructor in this package (HerbGrammar
) for a type defined in another package (HerbCore
). They could have been moved toHerbCore
as constructors, but then testing the constructors becomes difficult because they need aContextSensitiveGrammar
to test them, and that is defined in this package. So, in the end, it was just easier to change them to functions.