Additional QuantityArray constructions
#178
Conversation
icweaver
changed the title
QuantityArray constructions
Benchmark Results (Julia v1.10)Time benchmarks
Memory benchmarks
|
Benchmark Results (Julia v1)Time benchmarks
Memory benchmarks
|
|
Hi @MilesCranmer, this PR should be ready for your review now. Two follow-up questions I had were:
|
|
Sweet, looks like this is working nicely with the new Makie PR: julia> using CairoMakie, DynamicQuantities
julia> x = [6, 7, 8]us"cm"
3-element QuantityArray(::Vector{Float64}, ::Quantity{Float64, SymbolicDimensions{FixedRational{Int32, 25200}}}):
6.0 cm
7.0 cm
8.0 cm
julia> y = (4:6)u"kg"
3-element QuantityArray(::StepRangeLen{Float64, Base.TwicePrecision{Float64}, Base.TwicePrecision{Float64}, Int64}, ::Quantity{Float64, Dimensions{FixedRational{Int32, 25200}}}):
4.0 kg
5.0 kg
6.0 kg
julia> scatter(x, y)
|
Actually, think I found a good balance, just pushed. Does this look alright to you? |
|
Also re: your question, I think creating from generators is probably too much for now. |
Co-authored-by: Miles Cranmer <miles.cranmer@gmail.com>
Co-authored-by: Miles Cranmer <miles.cranmer@gmail.com>
|
Thanks Miles! Sorry for the mix-up with the README and generated index file; I think things have been straightened out now. In the process, I think I noticed that the HTML table of contents does not work as intended on my end at least, I think due to the case-sensitivity of the section headers. I'd be happy to open up a separate PR for this and maybe exploring the preprocessing step you mentioned for README code blocks if you think that would be useful |
|
Sure! Sounds great, thanks |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #178 +/- ##
==========================================
- Coverage 99.21% 95.62% -3.60%
==========================================
Files 21 20 -1
Lines 1273 1211 -62
==========================================
- Hits 1263 1158 -105
- Misses 10 53 +43 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
Added a to-do for the method ambiguities picked up by Aqua Note to self, related reading: |
|
Hi @MilesCranmer, definitely not the most elegant, but I think the remaining method ambiguities have been addressed now and that this PR is ready for another review |
|
Sorry is the Dates extension new? I don’t think I noticed that before. Could you put the Dates.jl extension in a separate PR please? Sorry for not mentioning earlier, I didn’t notice |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #178 +/- ##
=======================================
Coverage ? 94.99%
=======================================
Files ? 20
Lines ? 1219
Branches ? 0
=======================================
Hits ? 1158
Misses ? 61
Partials ? 0 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
docs/Project.toml
Outdated
| Documenter = "1" | ||
|
|
||
| [sources] | ||
| DynamicQuantities = {path = ".."} |
There was a problem hiding this comment.
DynamicQuantities doesn’t need to be specified here. (Also ideally this would go in a separate PR if possible, since it’s unrelated to the QuantityArray stuff)
There was a problem hiding this comment.
Oh yep, that snuck in by mistake, my bad. Looking forward to it being included in Abhro's PR =]
|
Hm I think the test coverage CI should fail, I think the repo change means codecov isn’t aware of the current coverage |
Yep, I introduced it in 096ca12 while playing method ambiguity whac-a-mole with Aqua. sg, will split into its own PR Updated: Started #181 |
Closes #166
To-do