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

User live score #78

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

Conversation

johnmadden86
Copy link
Contributor

tests still to be done

Copy link
Owner

@amosbastian amosbastian left a comment

Choose a reason for hiding this comment

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

All map and filter can be replaced by list comprehensions. Still need a solution for returning dictionaries breaking the previous version's code. Some other small changes as well, but this is what I had time to check for today.

fpl/fpl.py Outdated Show resolved Hide resolved
fpl/fpl.py Outdated Show resolved Hide resolved
fpl/fpl.py Show resolved Hide resolved
fpl/fpl.py Outdated Show resolved Hide resolved
fpl/fpl.py Outdated Show resolved Hide resolved
fpl/models/user.py Show resolved Hide resolved
fpl/models/user.py Show resolved Hide resolved
fpl/models/user.py Outdated Show resolved Hide resolved
fpl/models/user.py Outdated Show resolved Hide resolved
fpl/fpl.py Show resolved Hide resolved
@gringorichards
Copy link
Contributor

This feature to give us the user live score is great, I'm looking forward to it :)

@amosbastian
Copy link
Owner

I think you mentioned one of the reasons of returning dictionaries instead of lists was because it necessary for something you were going to implement, right? Could you point me to where exactly so I can see if there is a possible solution?

@johnmadden86
Copy link
Contributor Author

It's convenient as opposed to strictly necessary
e.g.
first_xi_live_scores = [players[player_id].live_score for player_id in first_xi]
here i can access the correct player directly just by calling the id, instead of performing an iteration

An alternative to be to make a "find" helper function that does this iteration.

Haven't looked at this for a a few days, but I was trying to tidy up my branches and make my master branch closer to yours so it'd be easier to merge. I had reverted to using lists for this - going to keep the dict returns in a separate branch - should eventually get it cleaned up properly
Take a look at my master branch and see how different it is now after my most recent push

@johnmadden86
Copy link
Contributor Author

johnmadden86 commented Nov 5, 2019

def find(items, id_): try: return items[id_] except KeyError: return next(item for item in items if item["id"] == id_)

This as a utility function would work for either dicts or lists
I'm not sure which is faster

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.

3 participants