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

Simplifies code in Data Providers and enhances API reference #335

Merged
merged 25 commits into from
Jun 13, 2024

Conversation

edoaltamura
Copy link
Contributor

Summary

Simplifies the usage in data providers and enhances documentation.

Details and comments

BaseDataProvider: eliminates code repetitions when checking for self._data and adds more documentation in API reference.
RandomDataProvider, WikipediaDataProvider, YahooDataProvider: adds more documentation in API reference.
RandomDataProvider: replaces unnecessary calls to Pandas with more readable Numpy functions.

✅ I have added the tests to cover my changes.
✅ I have updated the documentation accordingly.
✅ I have read the CONTRIBUTING document.

@CLAassistant
Copy link

CLAassistant commented Feb 24, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@woodsp-ibm
Copy link
Member

@edoaltamura This has been inactive for sometime, did you want to complete this off? You need at least to add from __future__ import annotations into some of these to allow the use of | in earlier Python versions.

@edoaltamura
Copy link
Contributor Author

I can make time to finalize it this coming week!

@coveralls
Copy link

coveralls commented Jun 11, 2024

Pull Request Test Coverage Report for Build 9467495943

Details

  • 49 of 75 (65.33%) changed or added relevant lines in 6 files are covered.
  • 4 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.08%) to 75.743%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_finance/data_providers/_base_data_provider.py 20 21 95.24%
qiskit_finance/data_providers/random_data_provider.py 15 16 93.75%
qiskit_finance/data_providers/yahoo_data_provider.py 6 7 85.71%
qiskit_finance/data_providers/wikipedia_data_provider.py 5 7 71.43%
qiskit_finance/data_providers/exchange_data_provider.py 2 10 20.0%
qiskit_finance/data_providers/data_on_demand_provider.py 1 14 7.14%
Files with Coverage Reduction New Missed Lines %
qiskit_finance/data_providers/_base_data_provider.py 1 85.33%
qiskit_finance/data_providers/exchange_data_provider.py 1 18.03%
qiskit_finance/data_providers/data_on_demand_provider.py 2 27.66%
Totals Coverage Status
Change from base Build 9155947037: -0.08%
Covered Lines: 612
Relevant Lines: 808

💛 - Coveralls

@coveralls
Copy link

coveralls commented Jun 12, 2024

Pull Request Test Coverage Report for Build 9479500352

Details

  • 49 of 75 (65.33%) changed or added relevant lines in 6 files are covered.
  • 7 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.04%) to 75.866%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_finance/data_providers/_base_data_provider.py 20 21 95.24%
qiskit_finance/data_providers/random_data_provider.py 15 16 93.75%
qiskit_finance/data_providers/yahoo_data_provider.py 6 7 85.71%
qiskit_finance/data_providers/wikipedia_data_provider.py 5 7 71.43%
qiskit_finance/data_providers/exchange_data_provider.py 2 10 20.0%
qiskit_finance/data_providers/data_on_demand_provider.py 1 14 7.14%
Files with Coverage Reduction New Missed Lines %
qiskit_finance/data_providers/_base_data_provider.py 1 85.33%
qiskit_finance/data_providers/exchange_data_provider.py 1 18.03%
qiskit_finance/data_providers/data_on_demand_provider.py 2 27.66%
qiskit_finance/data_providers/wikipedia_data_provider.py 3 64.58%
Totals Coverage Status
Change from base Build 9155947037: 0.04%
Covered Lines: 613
Relevant Lines: 808

💛 - Coveralls

woodsp-ibm
woodsp-ibm previously approved these changes Jun 12, 2024
Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

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

I made the comment above but as I said I think its ok. This is something that could always be improved in the future unless you'd like to course.

@edoaltamura
Copy link
Contributor Author

edoaltamura commented Jun 12, 2024

I updated the method bindings also in a couple of other places where relevant. I like working on the details! 🙂

@coveralls
Copy link

coveralls commented Jun 13, 2024

Pull Request Test Coverage Report for Build 9491153560

Details

  • 49 of 75 (65.33%) changed or added relevant lines in 6 files are covered.
  • 4 unchanged lines in 3 files lost coverage.
  • Overall coverage decreased (-0.08%) to 75.743%

Changes Missing Coverage Covered Lines Changed/Added Lines %
qiskit_finance/data_providers/_base_data_provider.py 20 21 95.24%
qiskit_finance/data_providers/random_data_provider.py 15 16 93.75%
qiskit_finance/data_providers/yahoo_data_provider.py 6 7 85.71%
qiskit_finance/data_providers/wikipedia_data_provider.py 5 7 71.43%
qiskit_finance/data_providers/exchange_data_provider.py 2 10 20.0%
qiskit_finance/data_providers/data_on_demand_provider.py 1 14 7.14%
Files with Coverage Reduction New Missed Lines %
qiskit_finance/data_providers/_base_data_provider.py 1 85.33%
qiskit_finance/data_providers/exchange_data_provider.py 1 18.03%
qiskit_finance/data_providers/data_on_demand_provider.py 2 27.66%
Totals Coverage Status
Change from base Build 9155947037: -0.08%
Covered Lines: 612
Relevant Lines: 808

💛 - Coveralls

Copy link
Member

@woodsp-ibm woodsp-ibm left a comment

Choose a reason for hiding this comment

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

Thanks for persisting with this. It all seems good to me now.

@woodsp-ibm woodsp-ibm merged commit 37b904d into qiskit-community:main Jun 13, 2024
16 checks passed
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.

4 participants