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 Cardano Account Pandas Dumper to Showcase #1141

Closed
wants to merge 10 commits into from

Conversation

pixelsoup42
Copy link

@pixelsoup42 pixelsoup42 commented Sep 4, 2023

Checklist

Showcase addition

  • Title: cardano_account_pandas_dumper
  • Description: cardano_account_pandas_dumper bridges Cardano transaction history with the Pandas data analysis framework, letting users create a report spreadsheet for their wallet's transaction history, including all native assets and many DeFI contracts.
  • Website: https://github.com/pixelsoup42/cardano_account_pandas_dumper
  • Source: <link_to_source_code>
  • Tags: analytics metadata nftsupport opensource

@pixelsoup42 pixelsoup42 changed the title Patch 1 Add cardano_account_pandas_dumper Sep 4, 2023
@rphair rphair changed the title Add cardano_account_pandas_dumper Add cardano_account_pandas_dumper to Showcase Sep 4, 2023
@rphair rphair added the showcase Indicates a PR/issue on showcase label Sep 4, 2023
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

@pixelsoup42 looks like a fair amount of work has gone into this & if it works I would be in favour of adding it.

Since this is a new GitHub account, with no visible track record for this project, we have to scrutinise it more than other submissions. I read over the code & it looks straightforward but I feel like there needs to be some explanation (in the code or README, not just here) of where the (impressive) list of exchange & DEX addresses came from.

Also: I'm trying to test as a Dev Portal user would, which has been difficult because of the "externally managed" Python environment beginning with Ubuntu 22.04 and other Linuxes following the same standard. Could you add something to the README explaining of how to install & run it through pipx or some other way of beating the external management?

$ pip install git+https://github.com/pixelsoup42/cardano_account_pandas_dumper
error: externally-managed-environment

@pixelsoup42
Copy link
Author

@pixelsoup42 looks like a fair amount of work has gone into this & if it works I would be in favour of adding it.

Since this is a new GitHub account, with no visible track record for this project, we have to scrutinise it more than other submissions. I read over the code & it looks straightforward but I feel like there needs to be some explanation (in the code or README, not just here) of where the (impressive) list of exchange & DEX addresses came from.

@rphair thanks a lot for the feedback.

Yeah I created this github account specifically for this tool without being aware of the requirement here for a not-new github account, changing everything would be kind of a hassle and defeat my original intent of keeping this cleanly separated. As you can see, this code does not need any secrets and is about 500 lines of Python, really easy to audit.

I gleaned the list of exchange and DEX addresses from external sources like cardanoscan.io and cexplorer.io, added that to the README.

As I also wrote in the README I'd be really glad to know about an API that would let me fetch this kind of info dynamically, could not find any.

Also: I'm trying to test as a Dev Portal user would, which has been difficult because of the "externally managed" Python environment beginning with Ubuntu 22.04 and other Linuxes following the same standard. Could you add something to the README explaining of how to install & run it through pipx or some other way of beating the external management?

$ pip install git+https://github.com/pixelsoup42/cardano_account_pandas_dumper
error: externally-managed-environment

Yeah using pipx is much better and simpler, I updated the pyproject.toml and README.md for that.

@rphair
Copy link
Collaborator

rphair commented Sep 4, 2023

pipx install git+https://github.com/pixelsoup42/cardano_account_pandas_dumper

pipx won't even find dependencies unless you specify an additional command option. People without Python or system admin experience will get that error and might not see they need to run again with the suggested option to install necessary dependencies.

cardano_account_pandas_dumper  --csv_output report.csv <staking_address1> <staking_address2> ...

Installed as above it doesn't create an executable anywhere.


I'm ready to approve this if you could first please think of Dev Portal users who will want to use your tool on a vanilla Ubuntu 22.04 installation or mainstream equivalent and have never used pipx and maybe not even pip or Python before. Please post again when you have feel you have something that works more-or-less as copy-and-paste.

@pixelsoup42
Copy link
Author

pipx install git+https://github.com/pixelsoup42/cardano_account_pandas_dumper

pipx won't even find dependencies unless you specify an additional command option. People without Python or system admin experience will get that error and might not see they need to run again with the suggested option to install necessary dependencies.

Sorry, I am confused, you mention an error in your reply but I don't see the error message in it.

cardano_account_pandas_dumper  --csv_output report.csv <staking_address1> <staking_address2> ...

Installed as above it doesn't create an executable anywhere.

I'm ready to approve this if you could first please think of Dev Portal users who will want to use your tool on a vanilla Ubuntu 22.04 installation or mainstream equivalent and have never used pipx and maybe not even pip or Python before. Please post again when you have feel you have something that works more-or-less as copy-and-paste.

Really sorry, it works just fine for me but I guess that's because I did the one-time pipx PATHsetup already .

Here is a script that works for me from a clean docker Debian image:

#!/bin/sh

docker run -i debian <<EOF
apt update >/dev/null 
apt install -y pipx git  >/dev/null 
yes | adduser --quiet test
su test
pipx install git+https://github.com/pixelsoup42/cardano_account_pandas_dumper
pipx ensurepath
bash -l -c 'cardano_account_pandas_dumper -h'
EOF

Does the above script work for you ? If so I will add the apt install -y pipx git, pipx ensurepath and re-login steps to the README.

@rphair
Copy link
Collaborator

rphair commented Sep 4, 2023

The first time I pipx install'd this tool it only put the script in a subdirectory of ~/.local/pipx/venvs and just the dependencies in ~/.local/bin. I had the now apparent misinformation that pipx ensurepath simply added ~/.local/bin to $PATH, so I didn't run it. 😕

I can see now pipx ensurepath does more than that, i.e. exposing the installed packages as executables. Therefore I think we'd be all set if you would please add that & the surrounding steps to your instructions in the README. 😎

Also for consistency with other Dev Portal listings where human readable titles are preferred unless nonexistent & to avoid potential line breaking problems.
@pixelsoup42
Copy link
Author

OK I have added this to the README:

Try to run cardano_account_pandas_dumper -h. If it fails with cardano_account_pandas_dumper: command not found, you need to run pipx ensurepath and open a new terminal window to use the tool.

@rphair rphair changed the title Add cardano_account_pandas_dumper to Showcase Add Cardano Account Pandas Dumper to Showcase Sep 4, 2023
Copy link
Collaborator

@rphair rphair left a comment

Choose a reason for hiding this comment

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

thanks @pixelsoup42 - works well for me and the revised installation instructions should work for users of various OS's and experience levels.

Further reviews (2 more approvals required) might dispute the use of the tags metadata and nftsupport which are not really the focus of the tool, though they're applied to wallets that are not focused on these 2 things either.

@rdlrt
Copy link
Collaborator

rdlrt commented Sep 5, 2023

Sorry - I'll pass for this one, a bit too simple JSON to spreadsheet conversion [with a bit to represent objects] for my eyes to be a showcase on dev portal in the tool's current state (others can approve and merge 👍🏻 )

@pixelsoup42
Copy link
Author

@rdlrt I don't think the target is devs specifically. There is nothing else that I know of to generate an account statement including all native assets and many DeFi positions, what else would you recommend ? Don't you think that can be useful for just any Cardano user, for tax purposes or other ?

@pixelsoup42
Copy link
Author

About the metadata and nftsupport tags: this tool parses transaction metadata, and will display NFT positions, that's why I added these tags.

@pixelsoup42
Copy link
Author

FWIW, in the showcase I see https://github.com/gitmachtl/cardano-signer which is about 1k lines of JS, and https://github.com/cardano-apexpool/token-registry-api which is about 700 lines of Python and some data.

@rphair
Copy link
Collaborator

rphair commented Sep 20, 2023

Just also wanted to add that I'm not a routine user of Blockfrost... nor do I really parse JSON routinely for personal reasons... and recently I've found myself using this utility in my own workflow even though there may be better & more portable options. If that is true for developers I can imagine how it would be of general interest to users.

One improvement I might throw in @pixelsoup42: when you accidentally paste a Cardano address instead of a stake key the warning returned isn't useful to that particular mistake. Users should be responsible for correcting that, but perhaps some logic could be added to detect common errors like that & help the user a bit... the current response is to suggest checking your API key from Blockfrost which could send new users down the wrong path. 🤔

@pixelsoup42
Copy link
Author

@rphair thanks a lot for the feedback. I have pushed a new version that adds staking rewards to the output, and supports graphical output with a timeline for each asset.
I have also improved error handling as you suggested, messages should be much more helpful now.
You can get the updated version with:

pipx upgrade cardano_account_pandas_dumper

@rphair
Copy link
Collaborator

rphair commented Oct 28, 2023

thanks for your attention @pixelsoup42 ... I'm still in favour of adding it 😎✅

@rphair
Copy link
Collaborator

rphair commented Nov 17, 2023

Just had to upgrade this tool to keep using it (new Blockfrost endpoint?); maybe @pixelsoup42 like the also Python hosted youtube-dl it could include a suggestion that an upgrade might fix the problem?

$ cardano_account_pandas_dumper --csv_output account.csv stake1...

Failed to read data from blockfrost.io: HTTPSConnectionPool(host='cardano-mainnet.blockfrost.io', port=443): Max retries exceeded with url ... (Caused by NameResolutionError(" ... Failed to resolve 'cardano-mainnet.blockfrost.io' ...

$ pipx upgrade cardano_account_pandas_dumper
upgraded package cardano-account-pandas-dumper from 2023.2.0 to 2023.2.3

BTW I'm still in favour of adding this to the Dev Portal, based on my own personal attachment to it. Maybe it's because I'm too lazy to formulate a Blockfrost query or massage the results but I do find myself using this regularly. cc @katomm @os11k @fill-the-fill

@rphair
Copy link
Collaborator

rphair commented Nov 17, 2023

(oops, hit "close with comment" when I meant to hit "cancel" button 😰)

@rphair rphair closed this Nov 17, 2023
@rphair rphair reopened this Nov 17, 2023
@pixelsoup42
Copy link
Author

@rphair Thanks for your feedback and support. I have added a "Troubleshooting" section to the README with your suggestions.

@katomm katomm closed this May 16, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
showcase Indicates a PR/issue on showcase
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants