Commit Samys12 POC #214
Commit Samys12 POC #214
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a new molecular building block set called "Samys12" for the Digital Molecule Maker application, expanding the available molecular components that users can combine.
Key changes include:
- Addition of 35 new molecular structure files and metadata
- Integration of the Samys12 block set into the application's services
- Updates to utility scripts to support the new block set generation
Reviewed Changes
Copilot reviewed 43 out of 99 changed files in this pull request and generated 8 comments.
Show a summary per file
| File | Description |
|---|---|
| src/assets/blocks/Samys12/mol2/*.mol2 | Molecular structure data files defining chemical compounds in MOL2 format |
| src/assets/blocks/Samys12/data.json | Metadata, properties, and molecular information for the Samys12 block set |
| src/app/services/block.service.ts | Service update to register and load the new Samys12 block set |
| scripts/utils/svg.py | Enhanced SVG processing utilities with better error handling and fallback config |
| scripts/utils/mol.py | Added Config class for block set management with Samys12 support |
| scripts/utils/json.py | Added Config class for JSON processing with Samys12 support |
| scripts/config.py | Updated default block set configuration to Samys12 |
| scripts/Samys12/generate.py | New script for generating Samys12 block set data from Excel input |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from xml.dom.minidom import parse, Text, Element | ||
|
|
||
| import numpy as np | ||
| from pathlib import Path # <-- added |
There was a problem hiding this comment.
Remove the inline comment '# <-- added' as it's unnecessary for production code and provides no meaningful documentation value.
| from pathlib import Path # <-- added | |
| from pathlib import Path |
| from tqdm import tqdm | ||
|
|
||
| from config import config | ||
| #from config import config |
There was a problem hiding this comment.
Remove the commented-out import statement since it has been replaced with a local Config class implementation.
| #from config import config |
| import os | ||
|
|
||
| from config import config | ||
| #from config import config |
There was a problem hiding this comment.
Remove the commented-out import statement since it has been replaced with a local Config class implementation.
| #from config import config |
|
|
||
| import numpy as np | ||
|
|
||
| # from config import config |
There was a problem hiding this comment.
Remove the commented-out import statement since it has been replaced with a local Config class implementation.
| # from config import config |
| def get_svg_dimensions(url: str): | ||
| # handle accidental leading "/" so we don't ignore src_dir | ||
| joined = os.path.join(config.src_dir, url.lstrip("/")) | ||
| #print(joined) |
There was a problem hiding this comment.
Remove the commented-out debug print statement or replace it with proper logging if debugging output is needed.
| #print(joined) |
| for filepath in glob.glob(os.path.join(config.workdir, "block_svg", "*.svg")): | ||
| process_svg(filepath, ["connection_in", "connection_out"]) | ||
|
|
||
| input_filepath = "/Users/amd/Downloads/blocks_samy.xlsx" |
There was a problem hiding this comment.
Hard-coded user-specific file path will fail on other systems. Move this to a command-line argument, configuration file, or relative path within the project structure.
| "cLogP": c_log_p, | ||
| } | ||
|
|
||
| import os, re, numpy as np # make sure these are imported at top of your file |
There was a problem hiding this comment.
Remove this duplicate import statement as these modules are already imported at the top of the file (lines 2, 5, and 9).
| import os, re, numpy as np # make sure these are imported at top of your file |
| (rx**2 * ry**2 - rx**2 * y1_**2 - ry**2 * x1_**2) | ||
| / ( | ||
| rx**2 * y1_**2 + ry**2 * x1_**2 | ||
| ), # this could become negative, probably due to rounding error |
There was a problem hiding this comment.
The inline comment is vague and doesn't explain the solution. Since the code uses max(0, ...) to handle negative values, update the comment to explain why this approach was chosen.
| ), # this could become negative, probably due to rounding error | |
| ), # Use max(0, ...) to guard against small negative values due to floating-point rounding errors, ensuring the square root receives a non-negative argument. |
No description provided.