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 inifinite loop #1166

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open

Fix inifinite loop #1166

wants to merge 4 commits into from

Conversation

andrii-uniq
Copy link

In case of absent mode paramater in the custom fsspec file system implementation removed rows caused infinite loop, as mentioned in comments several rows above

Copy link
Collaborator

@phofl phofl left a comment

Choose a reason for hiding this comment

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

Please open an issue clearly describing the problem and then add a test for your changes here. The current change is definitely incorrect since it will break existing stuff

@andrii-uniq
Copy link
Author

    def __getattr__(self, key):
        try:
            return object.__getattribute__(self, key)
        except AttributeError as err:
            if key.startswith("_meta"):
                # Avoid a recursive loop if/when `self._meta*`
                # produces an `AttributeError`
                raise RuntimeError(
                    f"Failed to generate metadata for {self}. "
                    "This operation may not be supported by the current backend."
                )
           ....
            if is_dataframe_like(self._meta) and key in self._meta.columns:
                return self[key]

I removed the last two rows. I don't know full library guts, so I can not correctly understand the logic behind the intended behavior. But the code clearly shows contradiction. Also, I have a specific example where these rows caused an infinite loop, but this is a rather specific case. If it is suitable, I can add a test for this case.

@phofl
Copy link
Collaborator

phofl commented Nov 14, 2024

the current logic definitely behaves as intended for what it was added for, there is no contradiction.

Yes, please add an example that is failing / causing an infinite loop

@andrii-uniq
Copy link
Author

I see:
# Avoid a recursive loop if/when self._meta*
if is_dataframe_like(self._meta)
Okay, I'll prepare a minimal example and will add it.

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