arrow-julia
arrow-julia copied to clipboard
Change scoped enums behavior
Implement suggestions for https://github.com/apache/arrow-julia/issues/308
I will to find time for that within a couple of days. But before I do so, should we reconsider the name of the primitive type, i.e. something other than with leading underscore? I just saw the announcement of enumx.jl https://discourse.julialang.org/t/ann-enumx-jl-improved-enums-for-julia/78096 , which has implemented a similar idea, but using T as the name of the type inside the module. E.g. for @scopedenum MyEnum::Base.UInt16 A=0 B=1 the values would be accessed as MyEnum.A and the primitive type could be accessed as MyEnum.T.
On second thought that T idea brakes down (without careful handleing) for enums containing T fields, which I guess are way more likely than _<enumname>.
edit:
I changed it to inner type name to __TYPE__ , as it is equal across different enums (which are identified by module name), and not likely to be an enum value name.
Codecov Report
Merging #309 (f5d485f) into main (4407c70) will not change coverage. The diff coverage is
79.61%.
@@ Coverage Diff @@
## main #309 +/- ##
=======================================
Coverage 87.15% 87.15%
=======================================
Files 26 26
Lines 3301 3301
=======================================
Hits 2877 2877
Misses 424 424
| Impacted Files | Coverage Δ | |
|---|---|---|
| src/metadata/File.jl | 30.61% <33.33%> (ø) |
|
| src/eltypes.jl | 86.55% <60.71%> (ø) |
|
| src/metadata/Message.jl | 77.68% <62.50%> (ø) |
|
| src/FlatBuffers/FlatBuffers.jl | 73.17% <75.00%> (ø) |
|
| src/metadata/Schema.jl | 84.35% <91.66%> (ø) |
|
| src/arraytypes/arraytypes.jl | 89.52% <100.00%> (ø) |
|
| src/arraytypes/bool.jl | 88.70% <100.00%> (ø) |
|
| src/arraytypes/compressed.jl | 100.00% <100.00%> (ø) |
|
| src/arraytypes/dictencoding.jl | 75.58% <100.00%> (ø) |
|
| src/arraytypes/fixedsizelist.jl | 84.53% <100.00%> (ø) |
|
| ... and 7 more |
Continue to review full report at Codecov.
Legend - Click here to learn more
Δ = absolute <relative> (impact),ø = not affected,? = missing dataPowered by Codecov. Last update 4407c70...f5d485f. Read the comment docs.
@quinnj Took me some time to update all usages of the enums in the package, but I got all tests to pass now.
I went ahead and rebased
@quinnj Is there something blocking getting this into main?
Would it be possible to replace the custom implementation of scoped enums here with a dependency on EnumX? That has no dependencies and is very small.
Sorry all that this languished; I've since become more familiar w/ the EnumX.jl package and agree it's the right way to go. I'll try to do a PR soon switching over to use it. Sorry @jonalm for all the headache here, but thanks for all the help along the way!