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

Adding a ct.get_all_results() implementation #1901

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

adirao-projects
Copy link

@adirao-projects adirao-projects commented Jan 7, 2024

Added an implementation of the get_all_results function desired in #1701 . The main issue with the current implementation I've added is that it requires either of the following.

  1. The user to provide a list of dispatch ids
  2. The user to provide a dispatcher and results directory and be running on their own machine

If these requirements are not met, the provided implementation will not work. The internal documentation I've provided explains the logic on how everything works. I have not yet updated the external documentation. No tests have been written to test this function yet.

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

@adirao-projects adirao-projects requested a review from a team as a code owner January 7, 2024 09:16
@CLAassistant
Copy link

CLAassistant commented Jan 7, 2024

CLA assistant check
All committers have signed the CLA.

Copy link

codecov bot commented Jan 7, 2024

Codecov Report

Attention: Patch coverage is 10.00000% with 18 lines in your changes missing coverage. Please review.

Project coverage is 84.89%. Comparing base (3e755ce) to head (49915cb).
Report is 37 commits behind head on develop.

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff             @@
##           develop    #1901      +/-   ##
===========================================
+ Coverage    84.49%   84.89%   +0.39%     
===========================================
  Files          295      181     -114     
  Lines        14472    11287    -3185     
  Branches       195        0     -195     
===========================================
- Hits         12228     9582    -2646     
+ Misses        2110     1705     -405     
+ Partials       134        0     -134     
Flag Coverage Δ *Carryforward flag
Dispatcher 92.33% <ø> (ø) Carriedforward from a574781
Functional_Tests ?
SDK 79.28% <10.00%> (-0.24%) ⬇️
UI_Backend ?
UI_Frontend ?

*This pull request uses carry forward flags. Click here to find out more.

@adirao-projects adirao-projects changed the title get_all_results implementation as defined in issue get_all_results implementation as defined in issue #1701 Jan 7, 2024
@adirao-projects adirao-projects changed the title get_all_results implementation as defined in issue #1701 Adding a ct.get_all_results() implementation Jan 7, 2024
Copy link
Author

@adirao-projects adirao-projects left a comment

Choose a reason for hiding this comment

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

Seems like this is the same as what was there before, but the bot thinks it's better format wise

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.

2 participants