Skip to content

WIP: Added instance method to generate MolBar from Structure#236

Open
crjacinto wants to merge 3 commits into
faccts:mainfrom
crjacinto:molbar
Open

WIP: Added instance method to generate MolBar from Structure#236
crjacinto wants to merge 3 commits into
faccts:mainfrom
crjacinto:molbar

Conversation

@crjacinto

@crjacinto crjacinto commented Apr 29, 2026

Copy link
Copy Markdown
Contributor

Closes Issues

Closes #227

Description

  • Added an instance method that generates the Molecular Barcode from Structure.

Release Notes

(Project adheres to Keep a Changelog; Every entry should start in upper-case and end with (#<pr-id>); Remove sections that don't apply)

Changed

  • Modified structure.py.

@crjacinto crjacinto requested a review from a team as a code owner April 29, 2026 13:04
@haneug haneug added enhancement New feature or request side input Concerning writing ORCA input labels Apr 30, 2026
@haneug

haneug commented Apr 30, 2026

Copy link
Copy Markdown
Contributor

@crjacinto please repair the title and the description of the PR

@crjacinto crjacinto changed the title Added class method to generate the Molecular Barcode (Molbar) from St… Added instance method to generate MolBar from Structure Class Apr 30, 2026
@crjacinto crjacinto changed the title Added instance method to generate MolBar from Structure Class Added instance method to generate MolBar from Structure Apr 30, 2026
@crjacinto

Copy link
Copy Markdown
Contributor Author

@crjacinto please repair the title and the description of the PR

Done

@haneug haneug left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice PR! Please split the function according to the return types and change the docstring to numpy style. Also an enum for the mode would be a good thing for verification I think.

Comment thread src/opi/input/structures/structure.py Outdated
Comment thread src/opi/input/structures/structure.py Outdated

Returns
-------
str

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Please split this into two separate functions, one returning a str, one returning tuple[str, dict[str, Any]. A function that changes its return type based on a keyword argument makes type hints unreliable and is easy to misuse. Maybe make to_molbar return the string and to_molbar_dict or to_molbar_full return the whole data.
Also please document or give the link to the documentation what this whole data even means.

Comment thread src/opi/input/structures/structure.py Outdated
Comment thread src/opi/input/structures/structure.py Outdated
elements.append(parts[0])
coords.append([float(parts[1]), float(parts[2]), float(parts[3])])

if not elements:

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

You could check for if self.atoms before trying to loop over them and then the error message would fit better.

Comment thread src/opi/input/structures/structure.py Outdated
Comment thread src/opi/input/structures/structure.py Outdated
Comment thread src/opi/input/structures/structure.py Outdated
Comment thread src/opi/input/structures/structure.py Outdated
Comment thread src/opi/input/structures/structure.py Outdated
@crjacinto crjacinto changed the title Added instance method to generate MolBar from Structure WIP: Added instance method to generate MolBar from Structure Jun 10, 2026
@crjacinto

crjacinto commented Jun 15, 2026

Copy link
Copy Markdown
Contributor Author

At this point, the latest commit of this code is compatible with the features developed for the RMSD-RotConst implementation (#230). The unit tests are failing since some of the features that are being used now come from the RMSD-RotConst code which has not yet been merged here. Once the other code is merged, I'm going to rebase this commit and it would be ready for review

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request side input Concerning writing ORCA input

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Simple integration of MolBar into OPI´s Structure Object

3 participants