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

add iter in FeedlyData #13

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

add iter in FeedlyData #13

wants to merge 2 commits into from

Conversation

lhoestq
Copy link
Contributor

@lhoestq lhoestq commented Jun 28, 2019

should fix #12

@Edouard360
Copy link
Collaborator

if 'content' in entry:
entry, instance of FeedlyData, is not supposed to be an iterable right ?
Rather, we should use the contains method that checks for membership in the _json ?

@lhoestq
Copy link
Contributor Author

lhoestq commented Jun 30, 2019

Well if we only have the contains overloading, then something like for i in entry would do a infinite loop. That's why iter overloading was ok because it also fixes this case

@Edouard360
Copy link
Collaborator

We're not supposed to do iter this way in the first place, are we ?
Wouldn't we rather raise an error if we wanted to take that edge case into account ? Maybe we could put an unit test for your case for i in entry ...

@kireet
Copy link
Contributor

kireet commented Jul 1, 2019

We're not supposed to do iter this way in the first place, are we ?
Wouldn't we rather raise an error if we wanted to take that edge case into account ? Maybe we could put an unit test for your case for i in entry ...

i think this would be a good solution...if people really want some dictionary like behavior on the data, they can call .json and use the underlying data?

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.

FeedlyData object doesn't iterate well.
3 participants