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

Add type annotations to xml.py #1327

Merged
merged 14 commits into from
Nov 22, 2023
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ default: tests

getdeps:
@echo "Installing required dependencies"
@pip install --user --upgrade autopep8 certifi pytest pylint urllib3 argon2-cffi pycryptodome
@pip install --user --upgrade autopep8 certifi pytest pylint urllib3 argon2-cffi pycryptodome typing-extensions

check: getdeps
@echo "Running checks"
Expand Down
67 changes: 51 additions & 16 deletions minio/xml.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,28 +16,36 @@

"""XML utility module."""

from __future__ import absolute_import
from __future__ import absolute_import, annotations

import io
from typing import Type, TypeVar
from xml.etree import ElementTree as ET

from typing_extensions import Protocol

_S3_NAMESPACE = "http://s3.amazonaws.com/doc/2006-03-01/"


def Element(tag, namespace=_S3_NAMESPACE): # pylint: disable=invalid-name
def Element( # pylint: disable=invalid-name
tag: str,
namespace: str = _S3_NAMESPACE,
) -> ET.Element:
"""Create ElementTree.Element with tag and namespace."""
return ET.Element(tag, {'xmlns': namespace} if namespace else {})
return ET.Element(tag, {"xmlns": namespace} if namespace else {})


def SubElement(parent, tag, text=None): # pylint: disable=invalid-name
def SubElement( # pylint: disable=invalid-name
parent: ET.Element, tag: str, text: str | None = None
) -> ET.SubElement:
"""Create ElementTree.SubElement on parent with tag and text."""
element = ET.SubElement(parent, tag)
if text is not None:
element.text = text
return element


def _get_namespace(element):
def _get_namespace(element: ET.Element) -> str:
"""Exact namespace if found."""
start = element.tag.find("{")
if start < 0:
Expand All @@ -49,7 +57,7 @@ def _get_namespace(element):
return element.tag[start:end]


def findall(element, name):
def findall(element: ET.Element, name: str) -> list[ET.Element]:
"""Namespace aware ElementTree.Element.findall()."""
namespace = _get_namespace(element)
return element.findall(
Expand All @@ -58,7 +66,7 @@ def findall(element, name):
)


def find(element, name):
def find(element: ET.Element, name: str) -> ET.Element | None:
"""Namespace aware ElementTree.Element.find()."""
namespace = _get_namespace(element)
return element.find(
Expand All @@ -67,7 +75,11 @@ def find(element, name):
)


def findtext(element, name, strict=False):
def findtext(
element: ET.Element,
name: str,
strict: bool = False,
) -> str | None:
"""
Namespace aware ElementTree.Element.findtext() with strict flag
raises ValueError if element name not exist.
Expand All @@ -80,20 +92,43 @@ def findtext(element, name, strict=False):
return element.text or ""


def unmarshal(cls, xmlstring):
K = TypeVar("K")
Copy link
Member

Choose a reason for hiding this comment

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

Have meaningful name here

Copy link
Contributor Author

@trim21 trim21 Nov 21, 2023

Choose a reason for hiding this comment

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

this is generic type parameter, it's custom to be named as T K J。it's like i x,y in looping.



class FromXmlType(Protocol):
"""typing stub for class with `fromxml` method"""

@classmethod
def fromxml(cls: Type[K], element: ET.Element) -> K:
"""Create python object with values from XML element."""


T = TypeVar("T", bound=FromXmlType)


def unmarshal(cls: Type[T], xmlstring: str) -> T:
"""Unmarshal given XML string to an object of passed class."""
return cls.fromxml(ET.fromstring(xmlstring))


def getbytes(element):
def getbytes(element: ET.Element) -> bytes:
"""Convert ElementTree.Element to bytes."""
data = io.BytesIO()
ET.ElementTree(element).write(
data, encoding=None, xml_declaration=False,
)
return data.getvalue()
with io.BytesIO() as data:
Copy link
Member

Choose a reason for hiding this comment

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

Why do we need to use with here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

close file and discard the buffer.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is not necessary, gc will handle them if we don't use with

Copy link
Member

Choose a reason for hiding this comment

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

Yep. For this function, it is unnecessary.

ET.ElementTree(element).write(
data,
encoding=None,
xml_declaration=False,
)
return data.getvalue()


class ToXmlType(Protocol):
"""typing stub for class with `toxml` method"""

def toxml(self, element: ET.Element | None) -> ET.Element:
"""Convert python object to ElementTree.Element."""


def marshal(obj):
def marshal(obj: ToXmlType) -> bytes:
"""Get XML data as bytes of ElementTree.Element."""
return getbytes(obj.toxml(None))
3 changes: 2 additions & 1 deletion setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@
long_description_content_type="text/markdown",
package_dir={"minio": "minio"},
packages=["minio", "minio.credentials"],
install_requires=["certifi", "urllib3", "argon2-cffi", "pycryptodome"],
install_requires=["certifi", "urllib3", "argon2-cffi",
"pycryptodome", "typing-extensions"],
tests_require=[],
license="Apache-2.0",
classifiers=[
Expand Down