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 float comparison in dicts #186

Merged
merged 3 commits into from
Sep 26, 2022
Merged

Allow float comparison in dicts #186

merged 3 commits into from
Sep 26, 2022

Conversation

hamogu
Copy link
Contributor

@hamogu hamogu commented Sep 22, 2022

Before this PR, floating numbers in arrays are compared as floating numbers, but floating numbers in dicts are not, because they end on "}". This PR simply adds "}" to the regexp that determines what can be at the end of a floating number to allows for doctests like

        >>> x = {'a': 1/3., 'b': 2/3.}
        >>> x    # doctest: +FLOAT_CMP
        {'a': 0.333333, 'b': 0.666666}

which failed before this PR.

Before this PR, floating numbers in arrays are compared as floating
numbers, but floating numbers in dicts are not, because they end on
"}". This PR simply adds "}" to the regexp that determines what can be
at the end of a floating number to allows for doctests like

            >>> x = {'a': 1/3., 'b': 2/3.}
            >>> x    # doctest: +FLOAT_CMP
            {'a': 0.333333, 'b': 0.666666}

which failed before this PR.
@hamogu hamogu mentioned this pull request Sep 22, 2022
@saimn saimn requested a review from a team September 23, 2022 10:08
Copy link
Contributor

@saimn saimn left a comment

Choose a reason for hiding this comment

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

Seems reasonable to me.

Copy link
Contributor

@pllim pllim left a comment

Choose a reason for hiding this comment

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

Oooo I didn't even know you can do this.

Do you need a new release?

@pllim
Copy link
Contributor

pllim commented Sep 26, 2022

CI was disabled due to inactivity. I am closing/reopening to trigger proper CI.

@pllim pllim closed this Sep 26, 2022
@pllim pllim reopened this Sep 26, 2022
@hamogu
Copy link
Contributor Author

hamogu commented Sep 26, 2022

I have not been following this repro, so I don't know how much development is happening. On the one hand, it seems a lot to to a release for exactly one added character in the code base excluding tests and changelogs [*], on the other hand, in sherpa/sherpa#1521 (comment) I've already decided to "just let my dict-comparison fail until pytest-doctestplus has a new release".

*: But I'm happy to compete for ""Smallest change that got it's own release

@pllim
Copy link
Contributor

pllim commented Sep 26, 2022

No other PRs are open and I don't see anyone fixing other issues anytime soon...

@pllim pllim merged commit 6fbe8cf into scientific-python:main Sep 26, 2022
@pllim
Copy link
Contributor

pllim commented Sep 26, 2022

@saimn , what do you think? Do we do a new release now?

@saimn
Copy link
Contributor

saimn commented Sep 26, 2022

I guess we can, it's supposed to be as simple as pushing a new tag :)

@pllim

This comment was marked as resolved.

@pllim
Copy link
Contributor

pllim commented Sep 26, 2022

@saimn
Copy link
Contributor

saimn commented Sep 26, 2022

Thanks !

@bsipocz
Copy link
Member

bsipocz commented Sep 26, 2022

More fixes and issues to come as we tests for upstream usability, but even then, doing incremental small releases is good.

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.

4 participants