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

Localization #57

Open
wants to merge 17 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -10,3 +10,6 @@ build
*.swo
*.pyc
.repos

.mypy_cache
mypy.ini
2 changes: 1 addition & 1 deletion .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ python:
- 3.6

install:
- pip install coveralls pycodestyle
- pip install coveralls pycodestyle pyyaml
- python setup.py install

script:
Expand Down
33 changes: 33 additions & 0 deletions src/blameandshame/components.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
class Component(object):

def __init__(self):
raise NotImplementedError


class File_C(Component):
Copy link
Contributor

Choose a reason for hiding this comment

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

Please can we change this to FileComponent?


def __init__(self,
filename: str = "") -> None:
self.filename = filename


class Line_C(Component):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above.


def __init__(self,
filename: str = "",
lineno: int = 0) -> None:
self.filename = filename
self.lineno = lineno


class Function_C(Component):
Copy link
Contributor

Choose a reason for hiding this comment

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

Same.


def __init__(self,
funcname: str = "",
filename: File_C = None,
line_start: int = 0,
line_end: int = 0) -> None:
self.funcname = funcname
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer hidden variables (e.g., self.__funcname) over public variables (e.g., self.funcname). Use the @property decorator to ensure read-only (external) access to the variable.

self.filename = filename
self.line_start = line_start
self.line_end = line_end
55 changes: 55 additions & 0 deletions src/blameandshame/localization.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
from typing import Dict, List, Type, TypeVar
from blameandshame.components import Component
import yaml

VERSION = "1.0"
G = TypeVar('G', bound=Component)
Copy link
Contributor

Choose a reason for hiding this comment

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

G and S are the same?

S = TypeVar('S', bound=Component)


class Localization (object):

@staticmethod
def from_file(filename: str) -> 'Localization':
"""
Builds a Localization object from a file
"""
with open(filename) as f:
try:
loc = yaml.load(f)
except KeyError as err:
print('Error when importing', filename, '.', err)
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this work? Prefer "foo {}; bar {}".format(x, y).

raise IOError
return loc
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the type of loc? Unless I've misunderstood, yaml.load(f) should return a dictionary? Does loc not need to be unpacked into a Localization object?


def to_file(self, filename: str) -> None:
"""
Stores its internal data in a file
"""
with open(filename, "w") as f:
yaml.dump(self, f)
Copy link
Contributor

Choose a reason for hiding this comment

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

Prefer to dump relevant information to a dictionary rather than dumping the entire object, which may contain irrelevant and/or ephemeral information.


def __init__(self,
mapping: Dict,
scope: List[S],
granularity: Type[G],
version: str = VERSION) -> None:

self.mapping = mapping
Copy link
Contributor

Choose a reason for hiding this comment

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

Use hidden variables and properties.

self.scope = scope
self.__version = version
self.__granularity = granularity

@property
def version(self) -> str:
"""
The version number of this localization object.
"""
return self.__version

@property
def granularity(self) -> Type[G]:
"""
The granularity of this object
"""
return self.__granularity
65 changes: 65 additions & 0 deletions tests/localization_test.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
#!/usr/bin/env python3
import tempfile
import textwrap
from typing import Dict, List, Type
import unittest
from blameandshame.localization import Localization
from blameandshame.components import Component, File_C, Function_C, Line_C


class LocalizationTestCase(unittest.TestCase):

def test_from_file(self):
"""
Tests the from_file function by creating a YAML file from a string
and using from_file to import it.
"""
def check_one(yaml_string: str,
granularity: Type[Component],
version: str,
mapping: Dict,
scope: List[Component]) -> None:
f = tempfile.NamedTemporaryFile()
f.write(yaml_string.encode())
f.seek(0)
l = Localization.from_file(f.name)
f.close()
self.assertEqual(l.granularity, granularity)
self.assertEqual(l.version, version)
self.assertEqual(l.mapping, mapping)
self.assertEqual(l.scope, scope)

simple_localization = textwrap.dedent(
"""
!!python/object:blameandshame.localization.Localization
_Localization__granularity: !!python/name:blameandshame.components.File_C
_Localization__version: '0.0'
mapping: {}
scope: []
"""
)
check_one(simple_localization, File_C, "0.0", {}, [])
simple_localization = textwrap.dedent(
"""
!!python/object:blameandshame.localization.Localization
_Localization__granularity: !!python/name:blameandshame.components.Function_C
_Localization__version: '0.0'
mapping: {}
scope: []
"""
)
check_one(simple_localization, Function_C, "0.0", {}, [])
simple_localization = textwrap.dedent(
"""
!!python/object:blameandshame.localization.Localization
_Localization__granularity: !!python/name:blameandshame.components.Line_C
_Localization__version: '0.0'
mapping: {}
scope: []
"""
)
check_one(simple_localization, Line_C, "0.0", {}, [])


if __name__ == '__main__':
unittest.main()