Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
[PDSM] Medical Testcases for benchmarking #157
[PDSM] Medical Testcases for benchmarking #157
Changes from 52 commits
5a8749e
4ecca83
b3b18a9
d79cb67
813e2c1
ddd8bf7
41ddf2a
29a62b8
3a38516
fb7c431
0f772de
fb7905d
827aad8
7b1d785
3356c9e
392da50
a0ca30a
9b4cf61
a3788b2
8dcc567
8b1321d
84abb7d
4caf83a
886c336
1768961
372e3c8
d731dba
e5be05f
20183cb
b177a7d
3dd8930
5e2d180
a53a8be
0a4b75a
d3c6fce
c853b72
f3a1597
4d67b3c
64e27d4
7ccd4f4
83b4905
ad7a0da
b48d03f
861282a
2b4f6ad
9f4e039
f871db0
74b821c
eb73efb
4a4fd38
4b53bb0
d4c433d
72c7429
be9cdc9
14490f1
5c10b8b
2714b9a
49e4e41
018191e
5328bd9
af82910
c137e4c
b1125cb
fffadab
8477af7
4caed92
5cb9e52
b3e2908
be8562e
d80efe6
b24b8d3
bf3808f
6ce37f0
8aeaaae
b73944e
c57d0ee
f5984db
0651824
dedc67f
6ff88de
37d0916
66293a9
a1f4807
ac5845f
112bd8d
e9bcc7b
f8393e8
99260b6
770802c
2cb47c9
70f7cc8
f40cae7
ee0482b
c1490eb
40086de
c7a4695
e753aec
ad2070d
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is not an intuitive name for the function. I had trouble understanding what it is for just from reading the code (what is a "wrong result file"). I would suggest naming the entire process something like "failure mode identification" such that it is intuitively clear what is happening. What you are doing is saving responses in case of a failure.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed the naming in c137e4c
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do we need
dotenv
? The implementation seems a bit hacky ;)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Might be a bit of a work around, but this was the only way we could implement the key loading. We have some questions, later per mail
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ytehran could you or another team member address this? prevents me from merging.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As mentioned, I would not do this. Rather use
.venv
and configure your setup to use this env reliably. Env setup is on the user, should not be mandated by the code.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@ytehran please someone remove this so we can merge this PR.