Add type annotations to ClusterSequence #87

ssrothman · 2022-06-13T10:28:54Z

This PR adds return type annotations to the ClusterSequence class, allowing code analysis tools such as pylance to understand fastjet methods. The previous behavior was that pylance read the abstract method definitions and concluded that the methods allows raised an error. This marked all code after a fastjet call as unreachable.

jpivarski · 2022-06-13T13:14:41Z

On the test failure:

Error: getCacheEntry failed: GitHub Actions is temporarily unavailable. Please visit https://www.githubstatus.com/ for the status of our services.

On the PR itself: I think this is a valuable thing, if it gets past a blocker. fastjet is more easily typeable than most Awkward Array-based projects because some of these methods can only return ak.Array, not ak.Record or a scalar. In more general applications, whether a given slice returns an array or a scalar (ak.Record is a kind of scalar) depends on the type of the arguments, which is not a distinction in Python types. (As far as Python is concerned, it's a difference in values, not types.)

Actually, there might be a few of these methods that, when they accept a single event (ak.Array of Momentum4D) as input, return a single number (scalar). Those cannot be annotated with -> ak.Array because whether they return an ak.Array or an int/float depends on whether the input ak.Array has Awkward type # * Momentum4D (single event) or # * var * Momentum4D (multiple events), which have the same Python type. @aryan26roy should scan the changes proposed in this PR to be sure that you're only annotating the cases that we know will always return ak.Array.

aryan26roy

Sorry for the late reply, I have been busy with a lot of stuff in the past 2 months. I have commented on all the methods that have been marked as returning an Awkward Array but can return a single value (bool, int or float) if the input is a single event type.

aryan26roy · 2022-07-26T16:40:33Z

src/fastjet/__init__.py

@@ -295,7 +295,7 @@ def constituents(self, min_pt=0):
        """
        raise AssertionError()

-    def exclusive_dmerge(self, njets=10):
+    def exclusive_dmerge(self, njets=10) -> ak.Array:


Can return a single number (single event).

aryan26roy · 2022-07-26T16:41:09Z

src/fastjet/__init__.py

@@ -306,7 +306,7 @@ def exclusive_dmerge(self, njets=10):
        """
        raise AssertionError()

-    def exclusive_dmerge_max(self, njets=10):
+    def exclusive_dmerge_max(self, njets=10) -> ak.Array:


Same problem.

aryan26roy · 2022-07-26T16:41:26Z

src/fastjet/__init__.py

@@ -317,7 +317,7 @@ def exclusive_dmerge_max(self, njets=10):
        """
        raise AssertionError()

-    def exclusive_ymerge_max(self, njets=10):
+    def exclusive_ymerge_max(self, njets=10) -> ak.Array:


Here as well.

aryan26roy · 2022-07-26T16:41:49Z

src/fastjet/__init__.py

@@ -328,7 +328,7 @@ def exclusive_ymerge_max(self, njets=10):
        """
        raise AssertionError()

-    def exclusive_ymerge(self, njets=10):
+    def exclusive_ymerge(self, njets=10) -> ak.Array:


aryan26roy · 2022-07-26T16:42:04Z

src/fastjet/__init__.py

@@ -339,7 +339,7 @@ def exclusive_ymerge(self, njets=10):
        """
        raise AssertionError()

-    def Q(self):
+    def Q(self) -> ak.Array:


aryan26roy · 2022-07-26T16:44:41Z

src/fastjet/__init__.py

@@ -422,7 +422,7 @@ def n_exclusive_subjets(self, data, dcut=0):
        """
        raise AssertionError()

-    def has_parents(self, data):
+    def has_parents(self, data) -> ak.Array:


Can return a single Bool.

aryan26roy · 2022-07-26T16:44:55Z

src/fastjet/__init__.py

@@ -433,7 +433,7 @@ def has_parents(self, data):
        """
        raise AssertionError()

-    def has_child(self, data):
+    def has_child(self, data) -> ak.Array:


Can return a single Bool.

aryan26roy · 2022-07-26T16:45:26Z

src/fastjet/__init__.py

@@ -444,7 +444,7 @@ def has_child(self, data):
        """
        raise AssertionError()

-    def jet_scale_for_algorithm(self, data):
+    def jet_scale_for_algorithm(self, data) -> ak.Array:


Can return a single number.

aryan26roy · 2022-07-26T16:46:01Z

src/fastjet/__init__.py

@@ -466,7 +466,7 @@ def unique_history_order(self):
        """
        raise AssertionError()

-    def n_particles(self):
+    def n_particles(self) -> ak.Array:


aryan26roy · 2022-07-26T16:46:22Z

src/fastjet/__init__.py

@@ -477,7 +477,7 @@ def n_particles(self):
        """
        raise AssertionError()

-    def n_exclusive_jets(self, dcut=0):
+    def n_exclusive_jets(self, dcut=0) -> ak.Array:


aryan26roy reviewed Jul 26, 2022

View reviewed changes

Add return type annotations

a232895

jmduarte force-pushed the main branch from fb24fe3 to a232895 Compare October 17, 2022 17:09

jmduarte added 2 commits October 18, 2022 06:28

use union; input types

dcb0625

pre-commit

f86fa76

jmduarte changed the title ~~Add return type annotations to ClusterSequence~~ Add type annotations to ClusterSequence Oct 18, 2022

Merge branch 'main' into main

7f45eb7

lgray merged commit a6a4726 into scikit-hep:main Oct 19, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add type annotations to ClusterSequence #87

Add type annotations to ClusterSequence #87

ssrothman commented Jun 13, 2022

jpivarski commented Jun 13, 2022

aryan26roy left a comment •

edited

Loading

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

aryan26roy Jul 26, 2022

Add type annotations to ClusterSequence #87

Add type annotations to ClusterSequence #87

Conversation

ssrothman commented Jun 13, 2022

jpivarski commented Jun 13, 2022

aryan26roy left a comment • edited Loading

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

aryan26roy left a comment •

edited

Loading