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

Fix printing of Interval object within Pandas.DataFrame #54

Closed
wants to merge 1 commit into from

Conversation

alexprincel
Copy link

This fixes issue brought up here pandas-dev/pandas#40081

@AlexandreDecan
Copy link
Owner

Thanks for the PR, and the explanations and pointers you provided in pandas-dev/pandas#40081.

I do not really like the idea of "neutralizing" __next__ just because of a display issue in pandas, esp. since neutralizing it means it will be recognized by hasattr. While I understand this is "on purpose" for the issue being discussed, it could have many side effects since __next__ is usually the way to identify support of the iterator protocol (and we do not support this protocol).

I'll wait a little bit before deciding on this PR. I would like first to see if I can find another solution, either in portion or perhaps even in pandas, since I think the "issue" is more on their side by bypassing the __repr__ protocol.

@AlexandreDecan
Copy link
Owner

I cannot find any other reasonable solution for this issue. An Interval is an iterable, not an iterator, and a such, I'm a bit reluctant to define a "dummy" __next__ method on it. I'm afraid of any potential side effect the presence of this method could have in practice. At the same time, I expect portion to be used in combination with pandas and the current way intervals are displayed is not only confusing but also misleading (since only the first atomic interval in a conjunction of atomic intervals will be actually displayed by pandas, therefore not even correctly representing the actual content of the conjunction).

@AlexandreDecan
Copy link
Owner

Inspired by your solution based on the presence of __next__, and not satisfied by the potential side effects it could have, I made the following ugly-but-safe hack in Interval:

    def __getattr__(self, name):
        if name == '__next__':
            # Hack for a better representation of intervals within pandas
            # See https://github.com/AlexandreDecan/portion/pull/54
            try:
                import inspect
                if inspect.stack()[1].function == 'pprint_thing':
                    return
            except Exception:
                pass
        raise AttributeError

It's ugly, for sure, but it's safe since we make sure it only (mostly) applies when call from pandas, does not affect any other call. Obviously, inspect is not safe to use in all implementations of Python (hence the try... except) but in that case, the current behaviour will apply.

What do you think? :)

@alexprincel
Copy link
Author

While hacky, I think you're right that this method is safer in other contexts where next is used for duck typing.

@AlexandreDecan
Copy link
Owner

I'll go that way then ;-)

Thank you!!

@AlexandreDecan AlexandreDecan added the enhancement New feature or request label May 12, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants