Skip to content

Add pool data from cardano node#170

Merged
LadyChristina merged 13 commits into
mainfrom
update_cardano_pool_data
Jun 11, 2026
Merged

Add pool data from cardano node#170
LadyChristina merged 13 commits into
mainfrom
update_cardano_pool_data

Conversation

@LadyChristina

@LadyChristina LadyChristina commented Jun 5, 2026

Copy link
Copy Markdown
Member

All Submissions:

  • Have you followed the guidelines in our Contributing documentation?
  • Have you verified that there aren't any other open Pull Requests for the same update/change?
  • Does the Pull Request pass all tests?

Description

Added pool metadata for Cardano that has been fetched from a full node and combined it with metadata from Big Query, so that the clustering is as complete as possible (it was observed that there were some clusters missing from the previous data, but this should be fixed now).

Checklist

Update Mapping Support Information Submissions:

  • For which ledger do you update the mapping information?
    • Cardano
  • What mapping information do you update?
    • identifiers
    • addresses
    • clusters
    • legal links
  • Did you update the tests (if needed)?

@LauraAntunes1 LauraAntunes1 left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Just a couple of comments but otherwise looks fine

Comment thread mapping_information/cardano_preprocessing/cardano_preprocessing.py Outdated
Comment thread mapping_information/cardano_preprocessing/cardano_preprocessing.py
return None

homepage_lower = homepage.lower()

@ZeeshanJan ZeeshanJan Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: We can also check if homepage_lower has a space in it.
It will help filtering out most invalid values included in INVALID_EXACT

@ZeeshanJan ZeeshanJan Jun 11, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggestion: Or may be, we can add another function to filter out invalid domain name before we match INVALID_EXACT:
Something on the lines:

def is_valid_url(url):
    parsed = urlparse(url)
    return parsed.scheme in ('http', 'https') and bool(parsed.netloc) and ' ' not in parsed.netloc

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Do you mean a trailing space or like a space in the middle? Because trailing spaces are handled on line 272 via strip(), and I'm not sure if spaces in the middle are a thing.

For the url validity it's a good idea but I was thinking we might want to allow for cases where they skip the prefix but are still valid pages, e.g. where the entry is mypage.com instead of https://mypage.com

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

I meant the spaces in between, for example coming soon and in process will automatically become invalid. A domain name does not have a space in it.

Yes, we can extend on URL validity to check if a TLD is present without http/https protocol. However, it may remain a manual check in the beginning.

Comment thread mapping_information/cardano_preprocessing/cardano_preprocessing.py

@ZeeshanJan ZeeshanJan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Looks good to me.
Please see a few suggestions I have left.

@LadyChristina LadyChristina merged commit faec971 into main Jun 11, 2026
1 check passed
@LadyChristina LadyChristina deleted the update_cardano_pool_data branch June 11, 2026 10:43
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