Skip to content

Conversation

@lohedges
Copy link
Contributor

@lohedges lohedges commented Oct 22, 2025

This PR closes #462 by fixing the name of the context manager. I've also updated the docstring to state that inputs need to use absolute paths when a working directory is set, i.e. since paths will no longer be relative to the working directory of the Python process.

Also closes #449 by fixing a typo in an RDKit d2d draw option. This worked previously, so either there was a typo in the RDKit API that has been fixed, or the RDKit code no longer allows invalid attributes to be set.

  • I confirm that I have merged the latest version of devel into this branch before issuing this pull request (e.g. by running git pull origin devel): [y]
  • I confirm that I have permission to release this code under the GPL3 license: [y]

@lohedges lohedges requested a review from mb2055 October 22, 2025 08:38
@lohedges lohedges added the bug Something isn't working label Oct 22, 2025
@lohedges lohedges temporarily deployed to biosimspace-build October 22, 2025 08:38 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build October 22, 2025 08:38 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build October 22, 2025 08:38 — with GitHub Actions Inactive
@lohedges lohedges temporarily deployed to biosimspace-build October 22, 2025 08:38 — with GitHub Actions Inactive
----------
dir : str
The path to the node library.
Copy link
Contributor

Choose a reason for hiding this comment

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

Would be good to clarify that the path to the node directory also needs to be an absolute path

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Instead I just store the node directory as an absolute path, so it works either way. (Obviously it will be returned as an absolute path too, but this isn't an issue.)

Copy link
Contributor

@mb2055 mb2055 left a comment

Choose a reason for hiding this comment

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

All good, but it would be good to also clarify that the path given in setNodeDirectory also needs to be absolute

@mb2055 mb2055 self-requested a review October 22, 2025 10:33
Copy link
Contributor

@mb2055 mb2055 left a comment

Choose a reason for hiding this comment

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

Great 👍 thanks

@lohedges lohedges merged commit 2cc1724 into devel Oct 22, 2025
@lohedges lohedges deleted the fix_462 branch October 22, 2025 10:53
lohedges added a commit that referenced this pull request Oct 22, 2025
lohedges added a commit that referenced this pull request Oct 22, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[BUG] Nodes fail to execute when run from within BioSimSpace [BUG] viewMapping function is currently broken

3 participants