sinogeom :moj plot #64
Conversation
src/io/fld-read.jl
Outdated
|
|
||
| return format, endian, bytes | ||
| dict = Dict([ |
There was a problem hiding this comment.
Compared to the current implementation, Using Dict might be bad in performance because Dict is mutable.
# after
julia> @btime datatype_fld_to_mat("byte")
1.822 μs (60 allocations: 3.81 KiB)
(UInt8, :be, 1)
# Before
julia> @btime datatype_fld_to_mat("byte")
50.033 ns (1 allocation: 32 bytes)
(UInt8, "ieee-be", 1)There was a problem hiding this comment.
Thanks very much for the info! Then I will avoid using Dict in performance critical places. Here it is just some IO for an obscure file type used rarely so I think this version is easier to read.
I'm mostly working to improve code coverage at the moment...
There was a problem hiding this comment.
BTW, is there a const variant that would be better to use? I couldn't find one...
There was a problem hiding this comment.
BTW, is there a const variant that would be better to use? I couldn't find one...
I don't know much about it, but I guess NamedTuple might work
There was a problem hiding this comment.
Thanks - good idea. Unfortunately the keys have - in the string so it would be some work. Maybe later...
Codecov Report
@@ Coverage Diff @@
## master #64 +/- ##
=======================================
Coverage 99.62% 99.62%
=======================================
Files 48 48
Lines 2120 2124 +4
=======================================
+ Hits 2112 2116 +4
Misses 8 8
Continue to review full report at Codecov.
|
|
I got accidentally got two branches mixed up but in the end this merge should just be sino_geom related. |
|
Yes, this is because you created this branch on the top of FYI, before #63 is merged, you can use Anyway, merge with squash always help fix this kind of issue :P another advantage of PR-and-merge routine 😄 |
and code formatting...