Train code split, Part 4: moving some of the model.py code into dataset.py #40
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
lotif
requested review from
ElahehBassak,
amrit110,
bzamanlooy,
fatemetkl,
masi-sh and
sarakodeiri
emersodb
reviewed
Sep 23, 2025
There was a problem hiding this comment.
Overall, I think this is a good start. Some of the documentation needs some additional work and details and there are some small code improvements that I think are worthwhile in this PR. Certainly a lot of work for us in improving the code readability over time as well 😂
emersodb
reviewed
Sep 24, 2025
emersodb
reviewed
Sep 24, 2025
|
@emersodb I have addressed all of your comments, please check again :) |
emersodb
approved these changes
Sep 26, 2025
bzamanlooy
pushed a commit
that referenced
this pull request
Oct 10, 2025
04d817f Trainer: Changing all literals to enums (#48) 33f9306 Adding ignore for pip 25.2 vulnerability, removing stale ones (#50) c8d30ca Remove mkdocs build dir, add to gitignore (#49) c01121c End-to-end Evaluation Script Example (#45) d231a4a Add Nearest Neighbor Distance Ratio and Epsilon Identifiability Privacy Metrics (#42) 53e423b Adding Mean F1 Score Difference and Hitting Rate Metrics (#39) 91150fd Adding in Hellinger and pMSE metrics (#38) 746644e Tightening Ruff Configuration (#46) 580d55f Adding data_split_ratios to both the diffusion config and the classifier config (#47) 72863be Refactoring core.logger into common.logger and removing it (#41) bef53bf Train code split, Part 4: moving some of the model.py code into dataset.py (#40) 80b0154 Upgrading pip to latest version to solve security issue (#44) 83beba6 New mypy flow and fixes to typing issues that were discovered (#43) 7e77f37 Train code split, Part 3: moving some of the model.py code into sampler.py (#9) git-subtree-dir: deps/midst-toolkit git-subtree-split: 04d817f
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
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.
PR Type
Fix
Short Description
Clickup Ticket(s): https://app.clickup.com/t/868ew1hg6
This is the last one of the code split tickets.
Moving data related code to
dataset.py, adding docstrings and type hints where missing and doing minor refactorings (major ones coming up next).Moving
data_loaders.pyto theclavaddpmfolder and adding missing docstrings and type hints in that file.Also adding missing docstrings and type hints in
model.py.Tests Added
NA