Fix magmap rank bug, magbin typo, plot.magbin speed; add tests and vignette sections#17
Draft
Copilot wants to merge 2 commits into
Draft
Fix magmap rank bug, magbin typo, plot.magbin speed; add tests and vignette sections#17Copilot wants to merge 2 commits into
Copilot wants to merge 2 commits into
Conversation
…dead code, add tests and vignette sections Agent-Logs-Url: https://github.com/asgr/magicaxis/sessions/74e9c01b-f592-4d54-8e81-353d57d8b91c Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>
Agent-Logs-Url: https://github.com/asgr/magicaxis/sessions/74e9c01b-f592-4d54-8e81-353d57d8b91c Co-authored-by: asgr <5617132+asgr@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Thorough audit of the package: two silent bugs, several doc/typo issues, a dead-code remnant, a speed improvement in the hot drawing loop, and materially expanded test and vignette coverage.
Bugs fixed
magmaptype='rank'produced no ranking (R/magmap.R)data[good][order(data[good])] = locut:hicutcreates an R temporary copy on the LHS —datawas never modified, so every point mapped identically. Replaced with vectorisedrank(..., ties.method='average').magbindistance deduplication was a no-op (R/magbin.Rline 226)output$output$nn.dists—output$outputisNULL, so the assignment silently did nothing. Fixed tooutput$nn.dists.Speed:
plot.magbinThe per-bin loop checked
x$shapeon every iteration (constant across all bins) and tested!is.na(colmap$map[i])in a conditional rather than pre-filtering. Refactored to select the drawing function once outside the loop and iterate only overwhich(!is.na(colmap$map)). Removed the now-redundant innercount > dustlimguard (bins are already filtered before the loop).Dead code removed
magband.R: unused.Last.magrojlocal object (a stale copy of.Last.magproj) removed.Documentation fixes
man/magclip.Rdman/magrun.Rdbincen/binlim→bincens/binlims(match code)man/magmap.RdTests added (
tests/testthat/test-functions.R)~50 new tests covering previously untested functions:
magclip,maghist,magrun(including the corrected return names),magerr,magcurve,magcon,magbar,magcutout, and a regression test for themagmaprank bug.Vignette additions
New worked-example sections in
vignettes/magicaxis.Rmd: magclip, magrun, magerr, magcurve, magbar.