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

Move ConverterBase out of coordinates/base.py #4253

Merged
merged 9 commits into from
Aug 29, 2023
5 changes: 4 additions & 1 deletion package/CHANGELOG
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ The rules for this file:
* release numbers follow "Semantic Versioning" http://semver.org

------------------------------------------------------------------------------
??/??/?? IAlibay
??/??/?? IAlibay, ianmkenney

* 2.7.0

Expand All @@ -22,8 +22,11 @@ Fixes
Enhancements

Changes
* ConverterBase class moved from coordinates/base.py to converters/base.py (Issue #3404)

Deprecations
* coordinates.base.ConverterBase has been deprecated and will be removed in 3.0.0;
use converters.base.ConvertBase instead (Issue #3404)


28/08/23 IAlibay, hmacdope, pillose, jaclark5, tylerjereddy
Expand Down
5 changes: 3 additions & 2 deletions package/MDAnalysis/converters/ParmEd.py
Original file line number Diff line number Diff line change
Expand Up @@ -81,13 +81,14 @@
import itertools
import warnings

from ..coordinates import base
from . import base
from ..coordinates.base import SingleFrameReaderBase
from ..topology.tables import SYMB2Z
from ..core.universe import Universe
from ..exceptions import NoDataError


class ParmEdReader(base.SingleFrameReaderBase):
class ParmEdReader(SingleFrameReaderBase):
"""Coordinate reader for ParmEd."""
format = 'PARMED'

Expand Down
3 changes: 2 additions & 1 deletion package/MDAnalysis/converters/RDKit.py
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,8 @@

import numpy as np

from ..coordinates import base, memory
from . import base
from ..coordinates import memory
from ..coordinates.PDB import PDBWriter
from ..core.topologyattrs import _TOPOLOGY_ATTRS
from ..exceptions import NoDataError
Expand Down
61 changes: 61 additions & 0 deletions package/MDAnalysis/converters/base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*-
# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4
#
# MDAnalysis --- https://www.mdanalysis.org
# Copyright (c) 2006-2017 The MDAnalysis Development Team and contributors
# (see the file AUTHORS for the full list of names)
#
# Released under the GNU Public Licence, v2 or any higher version
#
# Please cite your use of MDAnalysis in published work:
#
# R. J. Gowers, M. Linke, J. Barnoud, T. J. E. Reddy, M. N. Melo, S. L. Seyler,
# D. L. Dotson, J. Domanski, S. Buchoux, I. M. Kenney, and O. Beckstein.
# MDAnalysis: A Python package for the rapid analysis of molecular dynamics
# simulations. In S. Benthall and S. Rostrup editors, Proceedings of the 15th
# Python in Science Conference, pages 102-109, Austin, TX, 2016. SciPy.
# doi: 10.25080/majora-629e541a-00e
#
# N. Michaud-Agrawal, E. J. Denning, T. B. Woolf, and O. Beckstein.
# MDAnalysis: A Toolkit for the Analysis of Molecular Dynamics Simulations.
# J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787
#

""" Base classes --- :mod:`MDAnalysis.converters.base`
======================================================

Converters output information to other libraries.

.. autoclass:: ConverterBase
:members:
:inherited-members:
"""

from .. import _CONVERTERS
from ..coordinates.base import IOBase
from ..lib.util import asiterable


class _Convertermeta(type):
# Auto register upon class creation
def __init__(cls, name, bases, classdict):
type.__init__(type, name, bases, classdict)
try:
fmt = asiterable(classdict['lib'])
except KeyError:
pass
else:
for f in fmt:
f = f.upper()
_CONVERTERS[f] = cls


class ConverterBase(IOBase, metaclass=_Convertermeta):
IAlibay marked this conversation as resolved.
Show resolved Hide resolved
"""Base class for converting to other libraries.
"""

def __repr__(self):
return "<{cls}>".format(cls=self.__class__.__name__)

def convert(self, obj):
raise NotImplementedError
35 changes: 26 additions & 9 deletions package/MDAnalysis/coordinates/base.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*-
# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4
#

# MDAnalysis --- https://www.mdanalysis.org
# Copyright (c) 2006-2017 The MDAnalysis Development Team and contributors
# (see the file AUTHORS for the full list of names)
Expand Down Expand Up @@ -101,17 +101,18 @@
:members:
:inherited-members:


Converters
----------

Converters output information to other libraries.

.. deprecated:: 2.7.0
All converter code has been moved to :mod:`MDAnalysis.converters` and will
be removed from the :mod:`MDAnalysis.coordinates.base` module in 3.0.0.

.. autoclass:: ConverterBase
:members:
:inherited-members:


Helper classes
--------------

Expand All @@ -134,7 +135,7 @@
_READERS, _READER_HINTS,
_SINGLEFRAME_WRITERS,
_MULTIFRAME_WRITERS,
_CONVERTERS
_CONVERTERS, # remove in 3.0.0 (Issue #3404)
)
from .. import units
from ..auxiliary.base import AuxReader
Expand Down Expand Up @@ -1783,7 +1784,10 @@ def range_length(start, stop, step):
# The range is empty.
return 0


# Verbatim copy of code from converters/base.py
# Needed to avoid circular imports before removal in
# MDAnalysis 3.0.0
# Remove in 3.0.0
class _Convertermeta(type):
# Auto register upon class creation
def __init__(cls, name, bases, classdict):
Expand All @@ -1797,14 +1801,27 @@ def __init__(cls, name, bases, classdict):
f = f.upper()
_CONVERTERS[f] = cls


# Verbatim copy of code from converters/base.py
# Needed to avoid circular imports before removal in
# MDAnalysis 3.0.0
# Remove in 3.0.0
class ConverterBase(IOBase, metaclass=_Convertermeta):
Copy link
Member

Choose a reason for hiding this comment

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

We can't just purely move this out of here without some kind of deprecation window.

Could you maybe subclass the ConverterBase here and add a warning in __init_subclass__?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Unfortunately that leads to a circular reference. Instead I include the code in both base.py files but have a deprecation warning when the ConverterBase from coordinates is subclassed.

Copy link
Member

Choose a reason for hiding this comment

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

This solution works for me and the comments make clear what to do. I also really don't have a more elegant solution.

Copy link
Member

Choose a reason for hiding this comment

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

Ok that makes sense - we definitely need tests to check that each respective subclass does / doesn't raise the warning.

"""Base class for converting to other libraries.

See Also
--------
:mod:`MDAnalysis.converters`
.. deprecated:: 2.7.0
This class has been moved to
:class:`MDAnalysis.converters.base.ConverterBase` and will be removed
from :mod:`MDAnalysis.coordinates.base` in 3.0.0.
"""

def __init_subclass__(cls):
orbeckst marked this conversation as resolved.
Show resolved Hide resolved
wmsg = ("ConverterBase moved from coordinates.base."
"ConverterBase to converters.base.ConverterBase "
"and will be removed from coordinates.base "
"in MDAnalysis release 3.0.0")
warnings.warn(wmsg, DeprecationWarning, stacklevel=2)

def __repr__(self):
return "<{cls}>".format(cls=self.__class__.__name__)

Expand Down
8 changes: 8 additions & 0 deletions package/doc/sphinx/source/documentation_pages/converters.rst
Original file line number Diff line number Diff line change
Expand Up @@ -52,3 +52,11 @@ Another syntax is also available for tab-completion support::

core/accessors


Developers may find the :mod:`MDAnaylsis.converters.base` module helpful for
creating new converter classes.

.. toctree::
:maxdepth: 1

converters/base
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
.. automodule:: MDAnalysis.converters.base
56 changes: 56 additions & 0 deletions testsuite/MDAnalysisTests/converters/test_base.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# -*- Mode: python; tab-width: 4; indent-tabs-mode:nil; coding:utf-8 -*-
# vim: tabstop=4 expandtab shiftwidth=4 softtabstop=4 fileencoding=utf-8
#
# MDAnalysis --- https://www.mdanalysis.org
# Copyright (c) 2006-2017 The MDAnalysis Development Team and contributors
# (see the file AUTHORS for the full list of names)
#
# Released under the GNU Public Licence, v2 or any higher version
#
# Please cite your use of MDAnalysis in published work:
#
# R. J. Gowers, M. Linke, J. Barnoud, T. J. E. Reddy, M. N. Melo, S. L. Seyler,
# D. L. Dotson, J. Domanski, S. Buchoux, I. M. Kenney, and O. Beckstein.
# MDAnalysis: A Python package for the rapid analysis of molecular dynamics
# simulations. In S. Benthall and S. Rostrup editors, Proceedings of the 15th
# Python in Science Conference, pages 102-109, Austin, TX, 2016. SciPy.
# doi: 10.25080/majora-629e541a-00e
#
# N. Michaud-Agrawal, E. J. Denning, T. B. Woolf, and O. Beckstein.
# MDAnalysis: A Toolkit for the Analysis of Molecular Dynamics Simulations.
# J. Comput. Chem. 32 (2011), 2319--2327, doi:10.1002/jcc.21787

import pytest
import warnings


def test_coordinate_converterbase_warning():
from MDAnalysis.coordinates.base import ConverterBase
import MDAnalysis.converters.base

wmsg = ("ConverterBase moved from coordinates.base."
"ConverterBase to converters.base.ConverterBase "
"and will be removed from coordinates.base "
"in MDAnalysis release 3.0.0")

with pytest.warns(DeprecationWarning, match=wmsg):
class DerivedConverter(ConverterBase):
pass

assert issubclass(DerivedConverter, ConverterBase)
assert not issubclass(DerivedConverter,
MDAnalysis.converters.base.ConverterBase)


def test_converters_converterbase_no_warning():
from MDAnalysis.converters.base import ConverterBase

# check that no warning is issued at all
# when subclassing converters.base.ConverterBase
with warnings.catch_warnings():
warnings.simplefilter("error")

class DerivedConverter(ConverterBase):
pass

assert issubclass(DerivedConverter, ConverterBase)
Loading