Skip to content

Conversation

@sourcery-ai
Copy link
Contributor

@sourcery-ai sourcery-ai bot commented Mar 23, 2021

Branch develop refactored by Sourcery.

If you're happy with these changes, merge this Pull Request using the Squash and merge strategy.

See our documentation here.

Run Sourcery locally

Reduce the feedback loop during development by using the Sourcery editor plugin:

Review changes via command line

To manually merge these changes, make sure you're on the develop branch, then run:

git fetch origin sourcery/develop
git merge --ff-only FETCH_HEAD
git reset HEAD^

@sourcery-ai sourcery-ai bot requested a review from henryiii March 23, 2021 04:57
Comment on lines -117 to +121
if isinstance(result, tuple):
data, (edgesx, edgesy) = result
return data, edgesx, edgesy
else:
if not isinstance(result, tuple):
return result

data, (edgesx, edgesy) = result
return data, edgesx, edgesy
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function histogram2d refactored with the following changes:

Comment on lines -165 to +169
if isinstance(result, tuple):
data, (edges,) = result
return data, edges
else:
if not isinstance(result, tuple):
return result

data, (edges,) = result
return data, edges
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function histogram refactored with the following changes:

Comment on lines -348 to +354
elif options == {"circular", "underflow", "overflow"} or options == {
"circular",
"overflow",
}:
elif options in [
{"circular", "underflow", "overflow"},
{
"circular",
"overflow",
},
]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function Regular.__init__ refactored with the following changes:

Copy link
Member

@henryiii henryiii Mar 23, 2021

Choose a reason for hiding this comment

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

This should be a set. A set in a set... Has to be a frozenset... Well, okay.

Comment on lines -452 to +465
elif options == {"circular", "underflow", "overflow",} or options == {
"circular",
"overflow",
}:
elif options in [
{
"circular",
"underflow",
"overflow",
},
{
"circular",
"overflow",
},
]:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function Variable.__init__ refactored with the following changes:

Comment on lines -78 to +79
msg = "Developer shortcut: will be removed in a future version"
if isinstance(item, tuple) and len(item) == 3:
msg = "Developer shortcut: will be removed in a future version"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function _arg_shortcut refactored with the following changes:

Comment on lines 497 to 501
if self._hist.rank() == 1:
s = str(self._hist)
if self._hist.rank() != 1:
return repr(self)
s = str(self._hist)
# get rid of first line and last character
s = s[s.index("\n") + 1 : -1]
else:
s = repr(self)
return s
return s[s.index("\n") + 1 : -1]
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function Histogram.__str__ refactored with the following changes:

Comment on lines -550 to +555
self.axes = self._generate_axes_()

else: # Classic (0.10 and before) state
self._hist = state["_hist"]
self._variance_known = True
self.metadata = state.get("metadata", None)
for i in range(self._hist.rank()):
self._hist.axis(i).metadata = {"metadata": self._hist.axis(i).metadata}
self.axes = self._generate_axes_()

self.axes = self._generate_axes_()
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function Histogram.__setstate__ refactored with the following changes:

Comment on lines -782 to +785
else:
projections = [i for i in range(self.ndim) if i not in integrations]
projections = [i for i in range(self.ndim) if i not in integrations]

return (
self._new_hist(reduced.project(*projections))
if projections
else reduced.sum(flow=True)
)
return (
self._new_hist(reduced.project(*projections))
if projections
else reduced.sum(flow=True)
)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function Histogram.__getitem__ refactored with the following changes:

Comment on lines -114 to +123
if method == "__call__" and len(inputs) == 1:
if ufunc in {np.negative, np.positive}:
(result,) = kwargs.pop("out", [np.empty(self.shape, self.dtype)])
if (
method == "__call__"
and len(inputs) == 1
and ufunc in {np.negative, np.positive}
):
(result,) = kwargs.pop("out", [np.empty(self.shape, self.dtype)])

ufunc(inputs[0]["value"], out=result["value"], **kwargs)
result["variance"] = inputs[0]["variance"]
return result.view(self.__class__) # type: ignore
ufunc(inputs[0]["value"], out=result["value"], **kwargs)
result["variance"] = inputs[0]["variance"]
return result.view(self.__class__) # type: ignore
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function WeightedSumView.__array_ufunc__ refactored with the following changes:

Copy link
Member

Choose a reason for hiding this comment

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

Have to think about this one.

Comment on lines -17 to +23
if isinstance(name, str):
for i, ax in enumerate(self):
if ax.name == name:
return i
raise KeyError(f"{name} not found in axes")
else:
if not isinstance(name, str):
return name

for i, ax in enumerate(self):
if ax.name == name:
return i
raise KeyError(f"{name} not found in axes")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Function NamedAxesTuple._get_index_by_name refactored with the following changes:

@github-actions github-actions bot added the needs changelog Might need a changelog entry label Mar 23, 2021
@henryiii henryiii closed this Mar 23, 2021
@henryiii henryiii reopened this Mar 23, 2021
)
)
elif all((a == b or a == 1) for a, b in zip(other.shape, self.shape)):
elif all(a in [b, 1] for a, b in zip(other.shape, self.shape)):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
elif all(a in [b, 1] for a, b in zip(other.shape, self.shape)):
elif all(a in {b, 1} for a, b in zip(other.shape, self.shape)):

view = self.view(flow=False)
getattr(view, name)(other)
elif all((a == b or a == 1) for a, b in zip(other.shape, self.axes.extent)):
elif all(a in [b, 1] for a, b in zip(other.shape, self.axes.extent)):
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
elif all(a in [b, 1] for a, b in zip(other.shape, self.axes.extent)):
elif all(a in {b, 1} for a, b in zip(other.shape, self.axes.extent)):

@henryiii henryiii force-pushed the sourcery/develop branch 3 times, most recently from 33e3ee9 to 0635d90 Compare March 23, 2021 14:27
@sourcery-ai
Copy link
Contributor Author

sourcery-ai bot commented Mar 23, 2021

Sourcery Code Quality Report

✅  Merging this PR will increase code quality in the affected files by 0.23%.

Quality metrics Before After Change
Complexity 8.45 ⭐ 8.17 ⭐ -0.28 👍
Method Length 49.50 ⭐ 49.39 ⭐ -0.11 👍
Working memory 10.94 😞 10.94 😞 0.00
Quality 62.96% 🙂 63.19% 🙂 0.23% 👍
Other metrics Before After Change
Lines 2240 2249 9
Changed files Quality Before Quality After Quality Change
src/boost_histogram/numpy.py 41.64% 😞 41.90% 😞 0.26% 👍
src/boost_histogram/_internal/axis.py 75.53% ⭐ 75.67% ⭐ 0.14% 👍
src/boost_histogram/_internal/hist.py 55.57% 🙂 55.77% 🙂 0.20% 👍
src/boost_histogram/_internal/view.py 57.57% 🙂 58.38% 🙂 0.81% 👍
tests/test_accumulators.py 73.33% 🙂 72.59% 🙂 -0.74% 👎
tests/test_minihist_title.py 75.76% ⭐ 76.52% ⭐ 0.76% 👍

Here are some functions in these files that still need a tune-up:

File Function Complexity Length Working Memory Quality Recommendation
src/boost_histogram/_internal/view.py WeightedSumView.__array_ufunc__ 28 😞 430 ⛔ 23 ⛔ 14.07% ⛔ Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
src/boost_histogram/_internal/hist.py Histogram.__setitem__ 32 😞 325 ⛔ 21 ⛔ 14.81% ⛔ Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
src/boost_histogram/_internal/hist.py Histogram.__getitem__ 34 ⛔ 247 ⛔ 20 ⛔ 17.49% ⛔ Refactor to reduce nesting. Try splitting into smaller methods. Extract out complex expressions
src/boost_histogram/numpy.py histogramdd 16 🙂 264 ⛔ 19 ⛔ 28.12% 😞 Try splitting into smaller methods. Extract out complex expressions
src/boost_histogram/_internal/hist.py Histogram.fill 16 🙂 253 ⛔ 13 😞 35.05% 😞 Try splitting into smaller methods. Extract out complex expressions

Legend and Explanation

The emojis denote the absolute quality of the code:

  • ⭐ excellent
  • 🙂 good
  • 😞 poor
  • ⛔ very poor

The 👍 and 👎 indicate whether the quality has improved or gotten worse with this pull request.


Please see our documentation here for details on how these metrics are calculated.

We are actively working on this report - lots more documentation and extra metrics to come!

Let us know what you think of it by mentioning @sourcery-ai in a comment.

@henryiii henryiii merged commit a2a7205 into develop Mar 23, 2021
@henryiii henryiii deleted the sourcery/develop branch March 23, 2021 14:54
@henryiii henryiii removed the needs changelog Might need a changelog entry label Mar 23, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants