Feat: Base Class Implemenations by Nostrademous · Pull Request #17 · PathOfBuildingCommunity/PathOfBuilding-Python

Nostrademous · 2022-01-27T16:51:51Z

base implemenation of some classes following code_structures.md

ppoelzl · 2022-01-27T17:04:58Z

src/classes/Build.py

@@ -0,0 +1,33 @@
+#!/usr/bin/env python3


This does not work well for projects with external dependencies. The system Python interpreter is for system-level scripts. For your own applications, always use a virtual environment.

ppoelzl · 2022-01-27T17:07:24Z

src/classes/Build.py

+class Build:
+    def __init__(self, name: str = "temp") -> None:
+        self.name = name
+        self.tree_ref = Tree.Tree()


In Python, every variable is a reference to a value that lives somewhere else. Best to leave out the "ref" part, except when you would actually use weak references.

ppoelzl · 2022-01-27T17:14:55Z

src/classes/Build.py

+        self.tree_ref = Tree.Tree()
+        self.player_ref = Player.Player()
+
+    def __repr__(self) -> str:


__repr__ functions are intended for debugging only, not for human readable display. That's what __str__ is for. See https://docs.python.org/3/library/functions.html#repr and https://docs.python.org/3/library/functions.html#str

ppoelzl · 2022-01-27T17:17:15Z

src/classes/Build.py

@@ -0,0 +1,33 @@
+#!/usr/bin/env python3
+
+# Build Class


Consider replacing this redundant comment with a module-level docstring that describes teh purpose of this module.

ppoelzl · 2022-01-27T17:19:24Z

src/classes/Build.py

+
+# Build Class
+#
+# Contains:


For a table of contents, you could either use your IDE's conde structure view or extract the structure with an automatic documentation builder tool like Sphinx.

ppoelzl · 2022-01-27T17:20:49Z

src/classes/Player.py

+from enum import Enum
+
+
+class PlayerClasses(Enum):


Consider enum.IntEnum and enum.auto.

ppoelzl · 2022-01-27T17:22:46Z

src/classes/Player.py

+        self.player_class = player_class
+        self.ascendancy = ascendancy
+        self.level = level
+        self.stats = dict()


Consider dict literals.

ppoelzl · 2022-01-27T17:25:46Z

src/classes/Tree.py

+#     [dict] All Nodes (addressable by Node ID)
+#     [dict] Allocated Nodes (addressable by Node ID)
+
+_VERSION_ = "3.17"


If this should be a constant that is private to the module, remove the trailing underscore. Trailing underscores are usually uses to avoid name conflicts with built-ins and keywords.

ppoelzl · 2022-01-27T17:28:27Z

src/classes/Build.py

+        return ret_str
+
+
+def test() -> None:


Usually, tests are contained in their own file hierarcy, starting from tests/ at the repository root.

ppoelzl

Packages and modules should have lowercase names. See https://www.python.org/dev/peps/pep-0008/#package-and-module-names

ppoelzl · 2022-01-27T17:32:22Z

src/Enumerations.py

@@ -0,0 +1,17 @@
+# Enumeration Data for Path of Exile constants


This should be a module-level docstring.

ppoelzl · 2022-01-27T17:35:12Z

src/Player.py

+     [optional set] Minions
+"""
+
+from Enumerations import PlayerClasses, PlayerAscendancies


These enums are currently used in a single place. It's best to keep them as local as possible. When they are used by more than one module on the same package, put them in a separate module. When they are used by more than one package, consider reworking the packages or put them in their own package.

ppoelzl · 2022-01-27T17:36:22Z

src/Tree.py

+    [set] Allocated Nodes (addressable by Node ID)
+"""
+
+_VERSION_ = "3.17"


Consider storing version numbers as tuples.

Could you supply an example ? I'm too new to Python to understand what that means.

pHiney · 2022-06-25T12:54:27Z

Why are you using enumerations and not dictionaries

class PlayerClasses(Enum):
    SCION = 0
    MARAUDER = 1
    RANGER = 2
    WITCH = 3
    DUELIST = 4
    TEMPLAR = 5
    SHADOW = 6

I setup the colorCodes as a dictionary

colorCodes = {
    "NORMAL": "#000000",
    "MAGIC": "#8888FF",
    "RARE": "#FFFF77",

I'm not sure of the advantages of either.

X

ppoelzl · 2022-06-26T13:02:38Z

PEP 345 lists some reasons for using enums. Further advantages:

Enums have a distinct type
Dicts are stringly typed, so enums avoid typos
Enum variants are discoverable and you can auto-complete them
Nicer syntax: dot access vs key access; allows for automatic enumeration.
Stronger type guarantees for subtypes like enum.IntEnum (comparable to typing.TypedDict, but without the need for annotations)

pHiney · 2022-06-27T12:23:38Z

PEP 345 lists some reasons for using enums. Further advantages:

Currently I'm filling a combobox with

        self.colour_combo_box.addItems(color_codes.keys())

Which is super convenient.

Whilst the color_codes naturally would be good as enums, what hoops to to go through to fill the combobox ? Or is there a different way to choose colours ?

x

ppoelzl · 2022-06-27T15:09:06Z

You can iterate over an enum's variants and get their names:

self.colour_combo_box.addItems([colour.name for colour in ColourCodes])

base implemenation of some classes following code_structures.md

ff41f88

ppoelzl reviewed Jan 27, 2022

View reviewed changes

fix: moved 'classes' directly under src dir

3b78689

ppoelzl requested changes Jan 27, 2022

View reviewed changes

fix: minor fixups and documentation changes

32081d0

		@@ -0,0 +1,17 @@
		# Enumeration Data for Path of Exile constants

Conversation

Nostrademous commented Jan 27, 2022

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ppoelzl left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pHiney commented Jun 25, 2022

Uh oh!

ppoelzl commented Jun 26, 2022

Uh oh!

pHiney commented Jun 27, 2022

Uh oh!

ppoelzl commented Jun 27, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants