-
Notifications
You must be signed in to change notification settings - Fork 110
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
#2741 Image missing stereochemistry information when using abbreviations #2742
#2741 Image missing stereochemistry information when using abbreviations #2742
Conversation
Is is possible to write a unit test in api/cpp/tests/rendering/basic.cpp for this bug and others? These changes affect render_internal.cpp |
I guess in this particular case it is not render issue. It is just missing stereocenter, which is worth to test using just stereo information (e.g. number of stereo bonds down) |
Unfortunately we can't count the up and down bonds for the unit test; in this case, indigo correctly assigns the stereocenters with the correct bond directions, but when the molecule is passed through MoleculeRenderInternal::_prepareSGroups(), the stereocenter to the super atom is forgotten. |
for render tests we use regression test with pixel comparison
also, there are some requirements should be installed to python
|
please also merge latest changes from |
thanks for the info, I've added a test to render_reactions.py. I merged the changes from master yesterday. Git merge upstream/master is telling me I am up to date |
f5d1553
to
b8c8d91
Compare
Do you know what the problem is? My fork is in sync with indigo/master and my branch is up to date with my fork's master and the indigo/master branches |
I just verified, your branch is not up-to-date |
oh, I see, Indigo team didn't backmerge to master the changes about github actions. Will fix it and let you know |
please take the latest changes from master |
I don't have access to a linx/mac machine in order to generate the reference files for the regression test. How is this normally done? The windows tests fail due to a diff in the output - do I need to update a output reference file? |
|
thanks for the info - I've made the changes |
all tests passed, merged, thanks so much for the fix! |
Generic request
#1234 – issue name