Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow num_regression to accept array-like inputs #45

Merged
merged 5 commits into from
Jan 27, 2021

Conversation

jpn--
Copy link
Contributor

@jpn-- jpn-- commented Jan 27, 2021

Presently, the num_regression fixture only accepts actual 1-d numpy arrays. It would be convenient if it would also accept array-like inputs, including list of numbers and scalars. It's easy enough to convert them in my tests, but it's also super easy and suitably concise to make the change here. I'm not able to simply use the regular data_regression fixture in cases where the values are floats that might have a tiny bit of variance. The num_regression fixture with its tolerance control is exactly what I want in that instance.

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking great already, thanks @jpn-- for the contribution!

Please take a look at my two minor comments. 👍

@jpn-- jpn-- requested a review from nicoddemus January 27, 2021 12:18
Copy link
Member

@tadeu tadeu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome, thanks for this improvement :)

Copy link
Member

@nicoddemus nicoddemus left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot @jpn--!

@nicoddemus nicoddemus changed the title allow num_regression to accept array-like inputs Allow num_regression to accept array-like inputs Jan 27, 2021
@nicoddemus nicoddemus merged commit c6f80b1 into ESSS:master Jan 27, 2021
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.

3 participants