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

Re-write get_cli_statistics #463

Merged
merged 214 commits into from
Jan 19, 2024
Merged

Conversation

KatunaNorbert
Copy link
Member

Fixes #453.

idiom-bytes and others added 30 commits November 21, 2023 23:18
…_df() to verify that its working as intended. I believe kraken data is returning null at the moment.
* Towards #232: Refactoring towards ppss.yaml part 3/3
* move everything in model_eng/ to data_eng/
* Fix #352: [SW eng] High DRY violation in test_predictoor_agent.py <> test_predictoor_agent3.py
* Deprecate backend-dev.md (long obsolete), macos.md (obsolete due to vps), and envvars.md (obsolete because of ppss.yaml).
* Rename BaseConfig to web3_pp.py and make it yaml-based
* Move scripts into util/, incorporate them into pdr cli, some refactoring.
* revamp READMEs for cli. And, tighten up text for getting OCEAN & ROSE
* Deprecated ADDRESS_FILE and RPC_URL envvars.
* deprecate Predictoor approach 2. Pita to maintain 


Co-authored-by: trizin <25263018+trizin@users.noreply.github.com>
* Update check script CI

* Update cron topup

* Workflow dispatch

* Nevermind, revert previous commit

* Run on push to test

* Pass ppss.web3_pp instead of web3_config

* Don't run on push
* Add main.py back

* Black

* Linter

* Linter

* Remove "switch back to version v0.1.1"

* Black
trizin and others added 10 commits January 12, 2024 14:14
* Print texts: dftool -> pdrcli

* pdrcli -> pdr
* Make missing attributes message more friendly and integrate ai ss part to multimixin.
How fixed: use previous ascynio version.

Calina: Asyncio has some known issues, per their changelog. Namely issues with fixture handling etc., which I believe causes the warnings and test skips in our runs. They recommend using the previous version until they are fixed. It is also why my setup didn't spew up any warnings, my asyncio version was 21.1.

https://pytest-asyncio.readthedocs.io/en/latest/reference/changelog.html
* Fix web3_config.rpc_url in test_send_encrypted_tx

* Add conftest.py for system tests

* Add system test for get_traction_info

* Add system test for get_predictions_info

* Add system test for get_predictoors_info

* Add "PDRS" argument to _ArgParser_ST_END_PQDIR_NETWORK_PPSS_PDRS class

* Fix feed.exchange type conversion in publish_assets.py

* Add print statement for payout completion

* Add system level test for pdr topup

* Add conditional break for testing via env

* Add conditional break for testing via env

* Black

* Add test for pdr rose payout system

* System level test pdr check network

* System level test pdr claim OCEAN

* System level test pdr trueval agent

* Remove unused patchs

* Fix wrong import position in conftest.py

* Remove unused imports

* System level test for pdr dfbuyer

* System level tests for pdr trader

* System level tests for publisher

* Rename publisher test file

* Add conditional break in take_step() method

* Update dftool->pdr names in system tests

* Refactor test_trader_agent_system.py

* Add mock fixtures for SubgraphFeed and PredictoorContract

* Add system tests for predictoor

* Black

* Refactor system test files - linter fixes

* Linter fixes

* Black

* Add missing mock

* Add savefig assertion in test_topup

* Update VPS configuration to use development entry

* Patch verify_feed_dependencies

* Refactor test_predictoor_system.py to use a common test function

* Refactor trader approach tests to improve DRY

* Black

* Indent

* Ditch NETWORK_OVERRIDE

* Black

* Remove unused imports
Copy link

codeclimate bot commented Jan 16, 2024

Code Climate has analyzed commit 81dff47 and detected 3 issues on this pull request.

Here's the issue category breakdown:

Category Count
Complexity 1
Duplication 2

The test coverage on the diff in this pull request is 92.7% (50% is the threshold).

This pull request will bring the total coverage in the repository to 95.0% (0.0% change).

View more on Code Climate.

Base automatically changed from yaml-cli2 to main January 16, 2024 16:51
@idiom-bytes
Copy link
Member

idiom-bytes commented Jan 16, 2024

@KatunaNorbert I picked up on a couple more issues. You're almost there!

It looks like there are a bunch of conflicts to resolve as well. But, I'm sure you're aware of it.

Finally, please be diligent about how we merge this. I'm not sure how the PR will look but there are 205 commits related to this, as we're pointing from yaml-cli2 -> main

I am going to ask @calina-c for a review too so she can provide feedback to you. I know this is painful, but it will be better in-the-end as you get to take a PR to a high level of quality/coverage.

@idiom-bytes idiom-bytes requested a review from calina-c January 16, 2024 20:44
@KatunaNorbert
Copy link
Member Author

@calina-c I fixed the conflicts and added the change requested by @idiom-bytes

@idiom-bytes
Copy link
Member

idiom-bytes commented Jan 19, 2024

As part of this process we have also removed the logic for: get_endpoint_statistics + aggregate_prediction_statistics. I have updated the epic to document this and we can revisit this in the future.

Copy link
Member

@idiom-bytes idiom-bytes left a comment

Choose a reason for hiding this comment

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

Looks good!

@idiom-bytes idiom-bytes merged commit 0404493 into main Jan 19, 2024
5 checks passed
@idiom-bytes idiom-bytes deleted the issue453-rewrite-getCliStatistics branch January 19, 2024 05:06
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.

[Lake][Analytics] Re-write get_cli_statistics() into 2 fns and to use polars
6 participants