Skip to content

Added test file for mse and plot methods#2

Open
duffee wants to merge 2 commits into
jtrujil43:mainfrom
duffee:extend-test-coverage
Open

Added test file for mse and plot methods#2
duffee wants to merge 2 commits into
jtrujil43:mainfrom
duffee:extend-test-coverage

Conversation

@duffee
Copy link
Copy Markdown

@duffee duffee commented May 12, 2026

I've added a test file to cover the mse and plot methods of your module. Devel::Cover reports the Total Coverage has increased from 55% to 85%. (I can upload the html coverage report if you want). You can include the new test 04-plot_fit.t if you find it useful.

Since Perl is all about personal style, I'll explain where I've departed from your style and you can make up your own mind if it's good or not.

Line 14: I've used a closure or an anonymous sub instead of a subroutine and you can pass those around. Works the same and people seem to like them for some reason.

Line 37: I've used subtests to group logically connected tests. It also localizes scope so that your next set of tests is reset to a cleaner state.

Line 40: Is the Mean Squared Error really supposed to be zero? I've no idea and is probably correct on a simple fit, but if it's important to you, test it.

Line 63: I'm always adding tests so I call done_testing(); to tell the test when I've got to the end. It saves counting tests.

I like the PR method of talking about code because you can see the changes and put your comment right next to the line you're talking about.

@mohawk2
Copy link
Copy Markdown

mohawk2 commented May 23, 2026

Please don't use hardcoded files in the working directory, instead use tempfile like @jovantrujillo is doing in the demo. Another strategy is to have a tempdir, and use File::Spec->catfile to put stuff in it.

@duffee
Copy link
Copy Markdown
Author

duffee commented May 23, 2026

updated to use tempfile()

I hadn't spotted the Demos. Nice!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants