Skip to content

Conversation

@atravitz
Copy link
Contributor

@atravitz atravitz commented Dec 6, 2024

I re-arranged the data in openfecli/test/data/results to mimic the results from a quickrun submission where every replicate is separate. I'll use this dataset to test the solution to this issue

I included the notebook I used to generate this data since I expect we might want to use it again in the future. It's not polished, but I think it's good to have some record of where this test data came from.

@github-actions
Copy link

github-actions bot commented Dec 6, 2024

🚨 API breaking changes detected! 🚨

1 similar comment
@github-actions
Copy link

github-actions bot commented Dec 6, 2024

🚨 API breaking changes detected! 🚨

@atravitz atravitz changed the title create test data for quickrun submitted in parallel [WIP] create test data for quickrun submitted in parallel Dec 6, 2024
@atravitz atravitz force-pushed the add_parallel_submitted_test branch from 7ee21d2 to cf463cb Compare December 6, 2024 17:54
@github-actions
Copy link

github-actions bot commented Dec 6, 2024

🚨 API breaking changes detected! 🚨

2 similar comments
@github-actions
Copy link

github-actions bot commented Dec 6, 2024

🚨 API breaking changes detected! 🚨

@github-actions
Copy link

github-actions bot commented Dec 6, 2024

🚨 API breaking changes detected! 🚨

@atravitz atravitz requested a review from jthorton December 6, 2024 18:24
@github-actions
Copy link

github-actions bot commented Dec 6, 2024

🚨 API breaking changes detected! 🚨

@atravitz atravitz requested a review from mikemhenry December 6, 2024 18:27
@atravitz atravitz self-assigned this Dec 6, 2024
@atravitz atravitz changed the title [WIP] create test data for quickrun submitted in parallel create test data for quickrun submitted in parallel Dec 6, 2024
@atravitz atravitz marked this pull request as ready for review December 6, 2024 18:28
@IAlibay
Copy link
Member

IAlibay commented Dec 6, 2024

@atravitz can you confirm that you loaded these back into ProtocolResults objects and it reads fine?

@github-actions
Copy link

github-actions bot commented Dec 7, 2024

🚨 API breaking changes detected! 🚨

@codecov
Copy link

codecov bot commented Dec 7, 2024

Codecov Report

Attention: Patch coverage is 54.54545% with 5 lines in your changes missing coverage. Please review.

Project coverage is 92.79%. Comparing base (c2e4ee9) to head (dfb6d73).
Report is 149 commits behind head on main.

Files with missing lines Patch % Lines
openfecli/tests/commands/test_gather.py 54.54% 5 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1040      +/-   ##
==========================================
- Coverage   94.58%   92.79%   -1.79%     
==========================================
  Files         134      134              
  Lines        9973     9980       +7     
==========================================
- Hits         9433     9261     -172     
- Misses        540      719     +179     
Flag Coverage Δ
fast-tests 92.79% <54.54%> (?)
slow-tests ?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

Copy link
Member

@IAlibay IAlibay left a comment

Choose a reason for hiding this comment

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

Just the one thing, otherwise - if it loads back up properly - then it looks good to me.

Copy link
Collaborator

@jthorton jthorton left a comment

Choose a reason for hiding this comment

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

LGTM!

@atravitz
Copy link
Contributor Author

atravitz commented Dec 9, 2024

@atravitz can you confirm that you loaded these back into ProtocolResults objects and it reads fine?

Both the protocol_result and unit_results load correctly

@atravitz atravitz force-pushed the add_parallel_submitted_test branch from 8674a56 to dfb6d73 Compare December 9, 2024 16:50
@github-actions
Copy link

github-actions bot commented Dec 9, 2024

🚨 API breaking changes detected! 🚨

@atravitz atravitz merged commit 55c9924 into main Dec 9, 2024
10 of 11 checks passed
@atravitz atravitz deleted the add_parallel_submitted_test branch December 9, 2024 16:51
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