Skip to content

Conversation

kurtsansom
Copy link
Contributor

@kurtsansom kurtsansom commented May 12, 2021

For relative equals there is check in case the values are small.

This commit provides a relative equals that defaults to direct when the values are small.
e.g. max(relative_tolerance*abs(expected), tolerance)

If there is a better way to do this please advise.

Originally I tried to keep with the naming convention, but ran into trouble with the length of function names and therefore used RelMinEqual which I really don't like.

Another thing that need some more attention is I didn't modify the added test yet to evaluate the threshold. It would be good to get feedback that this is the right approach before doing more.

Copy link
Member

@tclune tclune left a comment

Choose a reason for hiding this comment

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

I'll await feedback on my comments.

Also note that when I spoke to the lawyer on Friday, she indicated that no CLA's have yet been submitted for pFUnit. (Am guessing it is still in the hands of your institutions laywers.) Just note that I cannot merge the PR until the lawyer on my end gives the green light. (sigh)

@kurtsansom
Copy link
Contributor Author

I decided to rename the function to approx to simplify the name length, and mirror the name use by pytest as there doesn't appear to be an equivalent in JUnit. I removed the relative difference calculation as providing the difference and calculated tolerance is sufficient to describe the criteria that is failing.

@tclune
Copy link
Member

tclune commented May 17, 2021

Looks like it would not be too difficult to introduce aliases, but I'd not ask you to implement it.

The intro would define a small dictionary whose keys are the annotations and the values are the Fortran procedure names. It would then intercept the usual expansion:

        parser.outputFile.write(fragment.format(match.group(1), match.group(2)))

@kurtsansom
Copy link
Contributor Author

I made a simple example on this branch.

https://github.com/kurtsansom/pFUnit/tree/assert_alias

map:

assert_alias_map = {'approx' : 'ApproxEqual'}

in class AtAssert(Action):

        if (match.group(1).lower() in self.alias_map.keys()):
            function_name = self.alias_map[match.group(1).lower()]
        else:
            function_name = match.group(1)

@tclune
Copy link
Member

tclune commented Jul 5, 2021

Same request as the other. Please change to PR onto develop branch.

@kurtsansom
Copy link
Contributor Author

I will shoot for getting to this by the end of the week.

@tclune tclune self-requested a review July 23, 2021 12:01
@tclune
Copy link
Member

tclune commented Nov 15, 2021

@kurtsansom - do you still intend to resubmit this as a PR onto develop?

@kurtsansom
Copy link
Contributor Author

kurtsansom commented Nov 23, 2021

@tclune Apologies this has fallen off my radar. It looks like I just missed fixing this by about a week.

@kurtsansom kurtsansom changed the base branch from main to develop November 23, 2021 16:43
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