-
Notifications
You must be signed in to change notification settings - Fork 17
exports-v2 - Updates from image review #1058
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
Merged
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
rabstejnek
approved these changes
Jul 2, 2024
Collaborator
rabstejnek
left a comment
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.
Changes look good! I had just one comment about a TODO left in the code, after that is addressed I think its good to merge.
rabstejnek
added a commit
that referenced
this pull request
Jul 2, 2024
* Epi export rewrite (#911) * preliminary epi rewrite * changes * changes * changes * fix * added back something accidentally deleted * moved code * fix test * remove old stuff * cleanup: * use vectorized timestamp conversion * minor formatting --------- Co-authored-by: Andy Shapiro <[email protected]> * rewrite riskofbias exports (#921) * preliminary epi rewrite * changes * changes * changes * fix * added back something accidentally deleted * moved code * fix test * remove old stuff * cleanup: * rewrite riskofbias exports * update naming for domain and metric --------- Co-authored-by: Daniel Rabstejnek <[email protected]> * Epimeta export rewrite (#922) * preliminary epi rewrite * changes * changes * changes * fix * added back something accidentally deleted * moved code * fix test * remove old stuff * cleanup: * epimeta export rewrite * remove obsolete code * changes * merge fix * update admin site to browse data pivot by evidence type --------- Co-authored-by: Andy Shapiro <[email protected]> * Invitro export rewrite (#958) * Convert invitro exports * Fix exporter datetime converter when datetime is None * Fix exports where there's no data * DTXSID in export should be None instead of useless dict if missing * add endpoint categories * Remove bloated dsstox dict from exports in favor of dtxsid --------- Co-authored-by: Andy Shapiro <[email protected]> * Animal export rewrite (#961) * Made animal model exports, began configuring exporters * first export largely done, second export mostly done * Some cleanups, finished fourth animal exporter * Changes after more testing * Fix tests * Remove remaining flat_complete_* methods * Cleanup * Update sql_display to accept dict * Add TODO to comment to easily find it * Rename exporters to match the flat file exporter class names * Add safeguard for qs accessing and fix groupby side effects (#982) * two updates * move to correct place * exports-v2 - Updates from image review (#1058) * remove duplicate rename calls * add ci calculation * update treatment period calculation * add back space; fix caused too many false positives in comparison * properly handle a 500 response from the server in a data pivot * fix epi logic from plot review * fix invitro export when category id is null; remove category id from export * add unique rob columns * use same method for categories * refactor into a reusable method * remove pytest warning "Marks applied to fixtures have no effect" * refactor; write tests * add tests * rewrite animal export test using same pattern * add udf to openapi * remove extra whitespace (will change visuals; known change) --------- Co-authored-by: Daniel Rabstejnek <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
I reviewed around 3000 visuals and found very minor differences in <1% of them. This PR addresses fixing the issues in that 1%, along with a few other bugs I found when investigating these differences. Here's a detailed list of changes I made:
treatment_periodtransformation method didn't appropriately transform text in a few cases; this was rewrittendf.loc[list_of_strings]) if columns are not unique; a get unique name method was added, and the study evaluation fetching code was updated to be reused across exportsiv_endpoint-category_idfrom invitro data pivot exportsUnrelated change: