Mandd/cpm red #52

mandd · 2024-10-15T17:29:59Z

Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

What are the significant changes in functionality due to this change request?

For Change Control Board: Change Request Review

The following review must be completed by an authorized member of the Change Control Board.

1. Review all computer code.
2. If any changes occur to the input syntax, there must be an accompanying change to the user manual.
3. Make sure the Python code and commenting standards are respected (camelBack, etc.) - See on the wiki for details.
4. Regression tests have to complete successfully.
5. If significant functionality is added, there must be tests added to check this. Tests should cover all possible options. Multiple short tests are preferred over one large test.
6. The merge request must reference an issue. If the issue is closed, the issue close checklist shall be done.

wangcj05

I have some minor comments for you to consider. @mandd

wangcj05 · 2024-11-27T20:45:50Z

src/CPM/PertMain2.py


 class Activity:
  """
    This is the base class for a single activity
    Extended from the original development of Nofar Alfasi
    Source https://github.com/nofaralfasi/PERT-CPM-graph
  """
-  def __init__(self, name, duration):
+  def __init__(self, name, duration, res=None, childs=None):


Add docstrings for all arguments in __init__ method

wangcj05 · 2024-11-27T20:46:29Z

src/CPM/PertMain2.py

    self.name = str(name)
    self.duration = duration
+    self.subActivities = []
+    self.belongsToCP = False
+    self.resources = res
+
+    self.startTime = None
+    self.endTime   = None
+
+    if childs is None:
+      self.childs = []
+    else:
+      self.childs = childs


please document all self.variable in this method.

wangcj05 · 2024-11-27T20:47:24Z

src/CPM/PertMain2.py

+    else:
+      self.childs = childs
+
+  def printToJsn(self):


function name is ok, but I think printToJson is better.

wangcj05 · 2024-11-27T20:50:28Z

src/CPM/PertMain2.py

+  def updateChilds(self, childs):
+    """
+      Method designed to assign the childs of an activity
+      @ In, None


add docstring for childs

wangcj05 · 2024-11-27T20:51:03Z

src/CPM/PertMain2.py

+    """
+      Method designed to assign the childs of an activity
+      @ In, None
+      @ Out, file in jason format


There is no return for this function. Please update the docstring here.

wangcj05 · 2024-11-27T21:06:53Z

src/CPM/PertMain2.py

+  """Partition a directed graph into a list of subgraphs that contain
+  only entirely supported or entirely unsupported nodes.
+  """


please update the docstring.

wangcj05 · 2024-11-27T21:07:05Z

src/CPM/PertMain2.py

+  supported_nodes = {n for n, d in G.nodes(data="node_type") if d == "supported"}
+  unsupported_nodes = {n for n, d in G.nodes(data="node_type") if d == "unsupported"}


camelBack here?

wangcj05 · 2024-11-27T21:07:25Z

src/CPM/PertMain2.py

+
+  if plotting:
+      # Plot the stripped graph with the edges removed.
+      _node_colors = [c for _, c in H.nodes(data="node_color")]


wangcj05 · 2024-11-27T21:07:50Z

src/CPM/PertMain2.py

+  )
+
+  # Collect all removed edges for reconstruction.
+  G_minus_H = nx.DiGraph()


camelBack here?

wangcj05 · 2024-11-27T21:08:16Z

tests/unit_tests/CPM/CPM.py

+"""
+
+#For future compatibility with Python 3
+from __future__ import division, print_function, unicode_literals, absolute_import


I guess this can be removed

mandd

@wangcj05: I think all comments have been addressed

mandd · 2025-01-20T20:50:48Z

src/CPM/PertMain2.py

+    """
+    for act in self.forwardDict:
+      if act.returnResources() not in self.resources.columns:
+        print("Activity " + str(act.returnName()) + " requires a resource that is not allowed: " + str(act.returnResources()))


added but needs to be discussed

mandd · 2025-01-20T20:51:41Z

src/CPM/PertMain2.py

+    """
+      Method designed to return the immediate successors of a node
+      @ In, node, activity, activity being queried
+      @ Out, list, list of activities that are immediate successors of "node"


mandd · 2025-01-20T20:52:08Z

src/CPM/PertMain2.py

+    """
+      Method designed to return the immediate predecessors of a node
+      @ In, node, activity, activity being queried
+      @ Out, list, list of activities that are immediate predecessors of "node"


mandd · 2025-01-20T20:53:43Z

src/CPM/PertMain2.py

+    """
+      Method designed to return the number of immediate predecessors of a node
+      @ In, node, activity, activity being queried
+      @ Out, int, number activities that are immediate predecessors of "node"


mandd · 2025-01-20T20:53:51Z

src/CPM/PertMain2.py

+      @ In, node, activity, activity to be queried
+      @ Out, list, list of activities


mandd · 2025-01-20T20:54:23Z

src/CPM/PertMain2.py

+  def deleteActivity(self,node):
+    """
+      Method designed to remove an activity from a schedule
+      @ In, node, activity, activity to be removed


mandd · 2025-01-20T20:58:36Z

src/CPM/PertMain2.py

+    @ In, path, list, list of activities
+    @ In, CP, list, list of activities


mandd · 2025-01-20T21:02:32Z

src/CPM/PertMain2.py

+    @ In, result, list, lis of subpath
+    @ In, temp_list, list, temporary list of
+    @ In, particular_list, list, list of element that mark a separation between sub-lists
+    @ Out, result, list, list of subpaths that are part "path" parallel to "CP"


mandd · 2025-01-20T21:22:34Z

src/CPM/PertMain2.py

+  """Partition a directed graph into a list of subgraphs that contain
+  only entirely supported or entirely unsupported nodes.
+  """


wangcj05

changes are good.

wangcj05 · 2025-01-27T17:07:20Z

changes are good, checklist is good.

mandd added 14 commits July 1, 2024 14:49

edits

a863380

edited src file and test file

697f316

cleaned files

c5f429a

cleaned files 2

b611002

further development

e8ae9a0

working subpath search

75bfc67

last edits before PL

4e22fd7

add resources to CPM model

01a6660

finalized schedule

2e27a9d

last fY24 edits

8ce20b5

cleaning PR 1

2257b69

cleaning PR 2

ba40d65

tests files

6e7472b

update test

525b4be

mandd changed the title ~~[WIP] Mandd/cpm red~~ Mandd/cpm red Oct 16, 2024

mandd added 11 commits October 16, 2024 08:54

delete whitespaces

4972e70

edits

c9ee28b

edits

b51f8de

edits

8c7e604

edits

fd1dc3f

edits

048d793

edits

de46cc8

edits

09f5002

edits

ab3f36a

edits

d1b9c94

edits

cefa376

mandd requested a review from wangcj05 October 16, 2024 20:09

wangcj05 requested changes Nov 27, 2024

View reviewed changes

mandd commented Jan 20, 2025

View reviewed changes

addressing comments

ea7c66d

delete whispaces

53c4625

wangcj05 approved these changes Jan 27, 2025

View reviewed changes

wangcj05 merged commit e401e28 into devel Jan 27, 2025
2 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Mandd/cpm red #52

Mandd/cpm red #52

mandd commented Oct 15, 2024 •

edited by wangcj05

Loading

wangcj05 left a comment

wangcj05 Nov 27, 2024

mandd Jan 16, 2025

wangcj05 Nov 27, 2024

mandd Jan 16, 2025

wangcj05 Nov 27, 2024

mandd Jan 16, 2025

wangcj05 Nov 27, 2024

mandd Jan 16, 2025

wangcj05 Nov 27, 2024

mandd Jan 16, 2025

wangcj05 Nov 27, 2024

mandd Jan 20, 2025

wangcj05 Nov 27, 2024

mandd Jan 17, 2025

wangcj05 Nov 27, 2024

mandd Jan 17, 2025

wangcj05 Nov 27, 2024

mandd Jan 17, 2025

wangcj05 Nov 27, 2024

mandd Jan 17, 2025

mandd left a comment

mandd Jan 20, 2025

mandd Jan 20, 2025

mandd Jan 20, 2025

mandd Jan 20, 2025

mandd Jan 20, 2025

mandd Jan 20, 2025

mandd Jan 20, 2025

mandd Jan 20, 2025

mandd Jan 20, 2025

wangcj05 left a comment

wangcj05 commented Jan 27, 2025

		supported_nodes = {n for n, d in G.nodes(data="node_type") if d == "supported"}
		unsupported_nodes = {n for n, d in G.nodes(data="node_type") if d == "unsupported"}

		@ In, node, activity, activity to be queried
		@ Out, list, list of activities

		@ In, path, list, list of activities
		@ In, CP, list, list of activities

Mandd/cpm red #52

Mandd/cpm red #52

Conversation

mandd commented Oct 15, 2024 • edited by wangcj05 Loading

Pull Request Description

What issue does this change request address? (Use "#" before the issue to link it, i.e., #42.)

What are the significant changes in functionality due to this change request?

For Change Control Board: Change Request Review

wangcj05 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mandd left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wangcj05 left a comment

Choose a reason for hiding this comment

wangcj05 commented Jan 27, 2025

mandd commented Oct 15, 2024 •

edited by wangcj05

Loading