Skip to content
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

TestRequiresRDKit.test_converter_requires_rdkit fails on Azure Windows and with Azure Linux 3.10 #4687

Closed
orbeckst opened this issue Aug 25, 2024 · 1 comment · Fixed by #4688

Comments

@orbeckst
Copy link
Member

In the CI for PR #4682 I observed a test failure for a RDKit-related test TestRequiresRDKit.test_converter_requires_rdkit in the Windows Azure runners

  • Azure_Tests Win-Python310-64bit-ful
  • Azure_Tests Win-Python312-64bit-full
  • Azure_Tests Win-Python310-64bit-full-wheel
  • Azure_Tests Linux-Python310-64bit-full-wheel

Not failing with the RDKit issue:

  • Azure_Tests Win-Python312-64bit-full-wheel
  • Azure_Tests Linux-Python312-64bit-full-wheel

The GitHub Linux and macOS runners do not fail this test.

Windows Python 3.12

https://dev.azure.com/mdanalysis/cefe1135-cd6a-4689-8710-15bd37ad81e6/_apis/build/builds/7149/logs/95

2024-08-23T15:10:56.7901745Z _______________ TestRequiresRDKit.test_converter_requires_rdkit _______________
2024-08-23T15:10:56.7902163Z [gw0] win32 -- Python 3.12.5 C:\hostedtoolcache\windows\Python\3.12.5\x64\python.exe
2024-08-23T15:10:56.7902438Z 
2024-08-23T15:10:56.7902727Z self = <MDAnalysisTests.converters.test_rdkit.TestRequiresRDKit object at 0x00000167D7E47590>
2024-08-23T15:10:56.7903027Z 
2024-08-23T15:10:56.7903319Z     def test_converter_requires_rdkit(self):
2024-08-23T15:10:56.7904060Z         u = mda.Universe(PDB_full)
2024-08-23T15:10:56.7904413Z         with pytest.raises(ImportError,
2024-08-23T15:10:56.7904801Z                            match="RDKit is required for the RDKitConverter"):
2024-08-23T15:10:56.7905187Z >           u.atoms.convert_to("RDKIT")
2024-08-23T15:10:56.7905353Z 
2024-08-23T15:10:56.7905686Z D:\a\1\s\testsuite\MDAnalysisTests\converters\test_rdkit.py:61: 
2024-08-23T15:10:56.7906060Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2024-08-23T15:10:56.7906557Z C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\MDAnalysis\core\accessors.py:204: in __call__
2024-08-23T15:10:56.7907339Z     return convert(*args, **kwargs)
2024-08-23T15:10:56.7908145Z C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\MDAnalysis\converters\RDKit.py:367: in convert
2024-08-23T15:10:56.7908564Z     mol = atomgroup_to_mol(ag, **kwargs)
2024-08-23T15:10:56.7908968Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2024-08-23T15:10:56.7909227Z 
2024-08-23T15:10:56.7909497Z ag = <AtomGroup with 1877 atoms>, NoImplicit = True, max_iter = 200
2024-08-23T15:10:56.7909872Z force = False
2024-08-23T15:10:56.7910020Z 
2024-08-23T15:10:56.7910337Z     @lru_cache(maxsize=2)
2024-08-23T15:10:56.7911014Z     def atomgroup_to_mol(ag, NoImplicit=True, max_iter=200, force=False):
2024-08-23T15:10:56.7911462Z         """Converts an AtomGroup to an RDKit molecule without coordinates.
2024-08-23T15:10:56.7911769Z     
2024-08-23T15:10:56.7912072Z         Parameters
2024-08-23T15:10:56.7912399Z         -----------
2024-08-23T15:10:56.7912689Z         ag : MDAnalysis.core.groups.AtomGroup
2024-08-23T15:10:56.7913049Z             The AtomGroup to convert
2024-08-23T15:10:56.7913379Z         NoImplicit : bool
2024-08-23T15:10:56.7913756Z             Prevent adding hydrogens to the molecule and allow bond orders and
2024-08-23T15:10:56.7914122Z             formal charges to be guessed from the valence of each atom.
2024-08-23T15:10:56.7914489Z         max_iter : int
2024-08-23T15:10:56.7914863Z             Maximum number of iterations to standardize conjugated systems.
2024-08-23T15:10:56.7915271Z             See :func:`_rebuild_conjugated_bonds`
2024-08-23T15:10:56.7915564Z         force : bool
2024-08-23T15:10:56.7915939Z             Force the conversion when no hydrogens were detected but
2024-08-23T15:10:56.7916344Z             ``NoImplicit=True``. Mostly useful for inorganic molecules.
2024-08-23T15:10:56.7916690Z         """
2024-08-23T15:10:56.7916936Z         try:
2024-08-23T15:10:56.7917250Z             elements = ag.elements
2024-08-23T15:10:56.7917580Z         except NoDataError:
2024-08-23T15:10:56.7917930Z             raise AttributeError(
2024-08-23T15:10:56.7918257Z                 "The `elements` attribute is required for the RDKitConverter "
2024-08-23T15:10:56.7918677Z                 "but is not present in this AtomGroup. Please refer to the "
2024-08-23T15:10:56.7919113Z                 "documentation to guess elements from other attributes or "
2024-08-23T15:10:56.7919521Z                 "type `help(MDAnalysis.topology.guessers)`") from None
2024-08-23T15:10:56.7919810Z     
2024-08-23T15:10:56.7920478Z         if "H" not in ag.elements:
2024-08-23T15:10:56.7921270Z             if force:
2024-08-23T15:10:56.7921710Z                 warnings.warn(
2024-08-23T15:10:56.7922002Z                     "No hydrogen atom found in the topology. "
2024-08-23T15:10:56.7922341Z                     "Forcing to continue the conversion."
2024-08-23T15:10:56.7922634Z                 )
2024-08-23T15:10:56.7923283Z             elif NoImplicit:
2024-08-23T15:10:56.7923530Z >               raise AttributeError(
2024-08-23T15:10:56.7923869Z                     "No hydrogen atom could be found in the topology, but the "
2024-08-23T15:10:56.7924514Z                     "converter requires all hydrogens to be explicit. You can use "
2024-08-23T15:10:56.7925398Z                     "the parameter ``NoImplicit=False`` when using the converter "
2024-08-23T15:10:56.7925837Z                     "to allow implicit hydrogens and disable inferring bond "
2024-08-23T15:10:56.7926150Z                     "orders and charges. You can also use ``force=True`` to "
2024-08-23T15:10:56.7926482Z                     "ignore this error.")
2024-08-23T15:10:56.7927064Z E               AttributeError: No hydrogen atom could be found in the topology, but the converter requires all hydrogens to be explicit. You can use the parameter ``NoImplicit=False`` when using the converter to allow implicit hydrogens and disable inferring bond orders and charges. You can also use ``force=True`` to ignore this error.
2024-08-23T15:10:56.7927487Z 
2024-08-23T15:10:56.7927888Z C:\hostedtoolcache\windows\Python\3.12.5\x64\Lib\site-packages\MDAnalysis\converters\RDKit.py:426: AttributeError
2024-08-23T15:10:56.7928188Z 
2024-08-23T15:10:56.7928426Z ---------- coverage: platform win32, python 3.12.5-final-0 -----------

Windows Python 3.10

https://dev.azure.com/mdanalysis/cefe1135-cd6a-4689-8710-15bd37ad81e6/_apis/build/builds/7149/logs/84

2024-08-23T14:47:45.9462056Z _______________ TestRequiresRDKit.test_converter_requires_rdkit _______________
2024-08-23T14:47:45.9462567Z [gw0] win32 -- Python 3.10.11 C:\hostedtoolcache\windows\Python\3.10.11\x64\python.exe
2024-08-23T14:47:45.9462830Z 
2024-08-23T14:47:45.9463276Z self = <MDAnalysisTests.converters.test_rdkit.TestRequiresRDKit object at 0x000001F061637AF0>
2024-08-23T14:47:45.9463599Z 
2024-08-23T14:47:45.9463945Z     def test_converter_requires_rdkit(self):
2024-08-23T14:47:45.9464354Z         u = mda.Universe(PDB_full)
2024-08-23T14:47:45.9464755Z         with pytest.raises(ImportError,
2024-08-23T14:47:45.9465135Z                            match="RDKit is required for the RDKitConverter"):
2024-08-23T14:47:45.9465559Z >           u.atoms.convert_to("RDKIT")
2024-08-23T14:47:45.9465820Z 
2024-08-23T14:47:45.9466200Z D:\a\1\s\testsuite\MDAnalysisTests\converters\test_rdkit.py:61: 
2024-08-23T14:47:45.9466718Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2024-08-23T14:47:45.9467407Z C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\MDAnalysis\core\accessors.py:204: in __call__
2024-08-23T14:47:45.9467916Z     return convert(*args, **kwargs)
2024-08-23T14:47:45.9468388Z C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\MDAnalysis\converters\RDKit.py:367: in convert
2024-08-23T14:47:45.9468892Z     mol = atomgroup_to_mol(ag, **kwargs)
2024-08-23T14:47:45.9469320Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
2024-08-23T14:47:45.9469551Z 
2024-08-23T14:47:45.9469965Z ag = <AtomGroup with 1877 atoms>, NoImplicit = True, max_iter = 200
2024-08-23T14:47:45.9470378Z force = False
2024-08-23T14:47:45.9470579Z 
2024-08-23T14:47:45.9470946Z     @lru_cache(maxsize=2)
2024-08-23T14:47:45.9471334Z     def atomgroup_to_mol(ag, NoImplicit=True, max_iter=200, force=False):
2024-08-23T14:47:45.9471808Z         """Converts an AtomGroup to an RDKit molecule without coordinates.
2024-08-23T14:47:45.9472213Z     
2024-08-23T14:47:45.9472567Z         Parameters
2024-08-23T14:47:45.9472884Z         -----------
2024-08-23T14:47:45.9473282Z         ag : MDAnalysis.core.groups.AtomGroup
2024-08-23T14:47:45.9473774Z             The AtomGroup to convert
2024-08-23T14:47:45.9474157Z         NoImplicit : bool
2024-08-23T14:47:45.9474535Z             Prevent adding hydrogens to the molecule and allow bond orders and
2024-08-23T14:47:45.9474997Z             formal charges to be guessed from the valence of each atom.
2024-08-23T14:47:45.9475409Z         max_iter : int
2024-08-23T14:47:45.9475838Z             Maximum number of iterations to standardize conjugated systems.
2024-08-23T14:47:45.9476242Z             See :func:`_rebuild_conjugated_bonds`
2024-08-23T14:47:45.9476645Z         force : bool
2024-08-23T14:47:45.9477059Z             Force the conversion when no hydrogens were detected but
2024-08-23T14:47:45.9477566Z             ``NoImplicit=True``. Mostly useful for inorganic molecules.
2024-08-23T14:47:45.9477935Z         """
2024-08-23T14:47:45.9478287Z         try:
2024-08-23T14:47:45.9478658Z             elements = ag.elements
2024-08-23T14:47:45.9478995Z         except NoDataError:
2024-08-23T14:47:45.9479387Z             raise AttributeError(
2024-08-23T14:47:45.9479817Z                 "The `elements` attribute is required for the RDKitConverter "
2024-08-23T14:47:45.9480281Z                 "but is not present in this AtomGroup. Please refer to the "
2024-08-23T14:47:45.9480685Z                 "documentation to guess elements from other attributes or "
2024-08-23T14:47:45.9481142Z                 "type `help(MDAnalysis.topology.guessers)`") from None
2024-08-23T14:47:45.9481543Z     
2024-08-23T14:47:45.9481910Z         if "H" not in ag.elements:
2024-08-23T14:47:45.9482237Z             if force:
2024-08-23T14:47:45.9482610Z                 warnings.warn(
2024-08-23T14:47:45.9483021Z                     "No hydrogen atom found in the topology. "
2024-08-23T14:47:45.9483443Z                     "Forcing to continue the conversion."
2024-08-23T14:47:45.9483775Z                 )
2024-08-23T14:47:45.9484144Z             elif NoImplicit:
2024-08-23T14:47:45.9484528Z >               raise AttributeError(
2024-08-23T14:47:45.9485373Z                     "No hydrogen atom could be found in the topology, but the "
2024-08-23T14:47:45.9485791Z                     "converter requires all hydrogens to be explicit. You can use "
2024-08-23T14:47:45.9486257Z                     "the parameter ``NoImplicit=False`` when using the converter "
2024-08-23T14:47:45.9486713Z                     "to allow implicit hydrogens and disable inferring bond "
2024-08-23T14:47:45.9487205Z                     "orders and charges. You can also use ``force=True`` to "
2024-08-23T14:47:45.9487576Z                     "ignore this error.")
2024-08-23T14:47:45.9488356Z E               AttributeError: No hydrogen atom could be found in the topology, but the converter requires all hydrogens to be explicit. You can use the parameter ``NoImplicit=False`` when using the converter to allow implicit hydrogens and disable inferring bond orders and charges. You can also use ``force=True`` to ignore this error.
2024-08-23T14:47:45.9488892Z 
2024-08-23T14:47:45.9489401Z C:\hostedtoolcache\windows\Python\3.10.11\x64\lib\site-packages\MDAnalysis\converters\RDKit.py:426: AttributeError
2024-08-23T14:47:45.9489724Z 
2024-08-23T14:47:45.9490143Z ---------- coverage: platform win32, python 3.10.11-final-0 ----------
@IAlibay
Copy link
Member

IAlibay commented Aug 25, 2024

This seems like a legitimate failure, the azure pipelines are picking up a leftover from the numpy 2.0 migration. Now that rdkit supports numpy 2.0, the import checks are failing in the wrong way.

yuxuanzhuang pushed a commit to yuxuanzhuang/mdanalysis that referenced this issue Sep 9, 2024
* Investigate rdkit issue

* Update azure-pipelines.yml

* fix numpy 2.0 import block

* fix imports
hmacdope pushed a commit that referenced this issue Sep 10, 2024
* Fixed high dimensional GroupBase indexing.

* fixed pep8 issues

* Removed sanitisation

* Fix #4687 -- rdkit values in azure CI (#4688)

* Investigate rdkit issue

* Update azure-pipelines.yml

* fix numpy 2.0 import block

* fix imports

* mark analysis.pca.PCA as not parallelizable (#4684)

- fix #4680
- PCA explicitly marked as not parallelizable (at least not with
  simple split-apply-combine)
- add tests
- update CHANGELOG

* disable gsd

* disable gsd in azure

* reduce timeout and set logical

* fix azure

* restore timeout to 200

---------

Co-authored-by: Matthew Davies <128810112+MattTDavies@users.noreply.github.com>
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>
RMeli pushed a commit to RMeli/mdanalysis that referenced this issue Oct 3, 2024
* Investigate rdkit issue

* Update azure-pipelines.yml

* fix numpy 2.0 import block

* fix imports
RMeli pushed a commit to RMeli/mdanalysis that referenced this issue Oct 3, 2024
* Fixed high dimensional GroupBase indexing.

* fixed pep8 issues

* Removed sanitisation

* Fix MDAnalysis#4687 -- rdkit values in azure CI (MDAnalysis#4688)

* Investigate rdkit issue

* Update azure-pipelines.yml

* fix numpy 2.0 import block

* fix imports

* mark analysis.pca.PCA as not parallelizable (MDAnalysis#4684)

- fix MDAnalysis#4680
- PCA explicitly marked as not parallelizable (at least not with
  simple split-apply-combine)
- add tests
- update CHANGELOG

* disable gsd

* disable gsd in azure

* reduce timeout and set logical

* fix azure

* restore timeout to 200

---------

Co-authored-by: Matthew Davies <128810112+MattTDavies@users.noreply.github.com>
Co-authored-by: Irfan Alibay <IAlibay@users.noreply.github.com>
Co-authored-by: Oliver Beckstein <orbeckst@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants