-
Notifications
You must be signed in to change notification settings - Fork 4
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
Onchain Graph Integration #12
base: main
Are you sure you want to change the base?
Conversation
@@ -225,3 +234,465 @@ def remove_unused_variables(document_ast, query): | |||
if query.count(_variable.variable.name.value) == 1: | |||
del document_ast.definitions[0].variable_definitions[_count] | |||
return print_ast(document_ast) | |||
|
|||
|
|||
def format_poaps_data(poaps, existing_user=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
as a good practise, it is good to give methods signature and description section.
Example:
def add_numbers(num_one: int, num_two:int) -> int:
"""
adds two numbers num_one and num_two and returns their sum
"""
return num_one+num_two
|
||
name = poap_event.get('eventName') | ||
content_value = poap_event.get('contentValue', {}) | ||
addresses = attendee.get('owner', {}).get('addresses', []) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will fail ifattendee = poap.get('attendee', {})
is None. You should check what the value type of attendee
is before calling a get on it
src/airstack/generic.py
Outdated
return recommended_users | ||
|
||
|
||
def format_farcaster_followings_data(followings, existing_user=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same comments as last function for this one format_poaps_data
.
|
||
|
||
def format_lens_followings_data(followings, existing_user=None): | ||
if existing_user is None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code duplication
if existing_user is None:
existing_user = []
recommended_users = existing_user.copy()
for following in followings:
existing_user_index = -1
for index, recommended_user in enumerate(recommended_users):
recommended_user_addresses = recommended_user.get('addresses', [])
if any(address in recommended_user_addresses for address in following.get('addresses', [])):
existing_user_index = index
break
mutual_follower = following.get('mutualFollower', {})
follower = mutual_follower.get(
'Follower', []) if mutual_follower is not None else []
follows_back = bool(follower[0]) if follower else False
if existing_user_index != -1:
follows = recommended_users[existing_user_index].get('follows', {})
recommended_users[existing_user_index].update({
**following,
'follows': {
**follows,
'followingOnLens': True,
'followedOnLens': follows_back
}
})
else:
recommended_users.append({
**following,
'follows': {
'followingOnLens': True,
'followedOnLens': follows_back
}
})
```
to be taken to another sub method and be used then
src/airstack/generic.py
Outdated
return recommended_users | ||
|
||
|
||
def format_token_received_data(data, _recommended_users=None): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code duplication to be removed in the below methods too!
src/airstack/onchain_graph.py
Outdated
self.api_client = AirstackClient(api_key=api_key) | ||
|
||
async def __fetch_poaps_data(self, address, existing_users=[]): | ||
user_poaps_event_ids_query = """ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
These queries to be moved to the configuration file. Let's keep the code file compact
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Also add docstring on methods level
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You've used double underscores to indicate that some methods are private (e.g., __fetch_poaps_data). While this is a convention in Python, it's important to note that it doesn't make the methods truly private; it just changes their name to include the class name to avoid accidental name clashes in subclasses. If privacy is a concern, you should use a single underscore (e.g., _fetch_poaps_data) to indicate that the method is for internal use and shouldn't be accessed directly from outside the class.
Applicable to all the other methods
src/airstack/onchain_graph.py
Outdated
else: | ||
poaps_data_response = await poaps_data_response.get_next_page | ||
else: | ||
print("Error: ", poaps_data_response.error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure if this code should sit in the src
folder as this too seems to be a part of examples
src/airstack/onchain_graph.py
Outdated
else: | ||
poap_holders_data_response = await poap_holders_data_response.get_next_page | ||
else: | ||
print("Error: ", poap_holders_data_response.error) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Avoid using prints if this is to be made part of the python library
src/airstack/onchain_graph.py
Outdated
|
||
return recommended_users | ||
|
||
async def __fetch_farcaster_followings(self, address, existing_users=[]): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is a significant amount of code duplication in the methods that fetch data for different types (e.g., __fetch_farcaster_followings, __fetch_lens_followings, etc.). Consider refactoring these methods to reduce redundancy by creating a more generic method that accepts parameters for the specific type of data to fetch.
@@ -0,0 +1,759 @@ | |||
""" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
comments applicable to the whole file.
- Avoid code duplication by creating generic methods
- Add error handling
- Try using configuration or env variables instead of hard coded values
- migrate graphql queries to a separate file
- Pagination handling: ensure that you don't make too many requests in a short period of time to avoid rate limiting
- Removed unused code
- Remove print statements if this is a library code
No description provided.