Add `flake8-multiline-containers` #290

fealho · 2021-11-10T07:00:25Z

As part of #278, add flake8-multiline-containers and update the code accordingly.

codecov-commenter · 2021-11-10T07:01:01Z

Codecov Report

Merging #290 (018140b) into issue-278-base (88909fe) will not change coverage.
The diff coverage is 0.00%.

@@               Coverage Diff               @@
##           issue-278-base     #290   +/-   ##
===============================================
  Coverage           87.25%   87.25%           
===============================================
  Files                  27       27           
  Lines                1702     1702           
===============================================
  Hits                 1485     1485           
  Misses                217      217

Impacted Files	Coverage Δ
copulas/bivariate/__init__.py	`96.96% <ø> (ø)`
copulas/bivariate/base.py	`86.89% <ø> (ø)`
copulas/bivariate/clayton.py	`84.78% <ø> (ø)`
copulas/bivariate/gumbel.py	`87.75% <ø> (ø)`
copulas/multivariate/tree.py	`96.96% <0.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 88909fe...018140b. Read the comment docs.

into issue-278-multiline

fealho · 2021-11-17T15:23:45Z

tests/unit/multivariate/test_tree.py

@@ -655,18 +655,18 @@ def test_get_likelihood_no_parents(self, bivariate_mock):

        expected_partial_derivative_call_args = [
            (
-                (np.array([[
+                (np.array([[  # noqa: JS101


@csala I'm not sure what to do about these. If I do follow the lint rule, I have to do the following:

( np.array( [ [ [0.25, 0.75], [0.50, 0.50], ] ] ), ), {}

which looks silly.

Yeah it is a bit excessive, but my preference would be to do the formatting over ignoring the rule here.

I agree, it's a bit excessive, and I also agree that in general it's better to re-format than to ignore the rule. It's also happening in only two places, so it's not that bad to have it fully expanded.

Done. I also created intermediate variables for them because the flake8-expression-complexity addon complains about these lines in a different PR.

fealho · 2021-11-17T15:29:27Z

copulas/bivariate/base.py

-        The generator is a function :math:`\psi: [0,1]\times\Theta \rightarrow [0, \infty)`
-        that given an Archimedian copula fulills:
+        The generator is a function
+        :math:`\psi: [0,1]\times\Theta \rightarrow [0, \infty)`  # noqa: JS101


The reason for this noqa is that [0, \infty) is interpreted as an opening [ without closing bracket. I'm not sure how to avoid this misinterpretation, so I just noqa'd it.

fealho · 2021-11-17T15:29:37Z

copulas/bivariate/clayton.py

-        The generator is a function :math:`\psi: [0,1]\times\Theta \rightarrow [0, \infty)`
-        that given an Archimedian copula fulills:
+        The generator is a function
+        :math:`\psi: [0,1]\times\Theta \rightarrow [0, \infty)`  # noqa: JS101


Same as above.

fealho · 2021-11-17T15:30:49Z

copulas/bivariate/gumbel.py

-                    &= \frac{C(u,v)}{uv} \frac{((-\ln u)^{\theta} + (-\ln v)^{\theta})^{\frac{2}
-                {\theta} - 2 }}{(\ln u \ln v)^{1 - \theta}} ( 1 + (\theta-1) \big((-\ln u)^\theta
+                    &= \frac{\partial^2 C(u,v)}{\partial v \partial u}
+                    &= \frac{C(u,v)}{uv} \frac{((-\ln u)^{\theta}  # noqa: JS101


These are multiline fractions. Since I couldn't find a way to write them in one line, I'm using noqa.

fealho · 2021-11-17T15:31:54Z

copulas/bivariate/__init__.py

@@ -156,7 +156,7 @@ def select_copula(X):
        np.concatenate((left, right)) for left, right in zip(
            candidate_left_auts, candidate_right_auts
        )
-    ]
+    ]  # noqa: JS102


I don't know why it complains about this one...

That's strange. Would it help to add commas? e.g.

candidate_auts = [ np.concatenate((left, right)) for left, right in zip( candidate_left_auts, candidate_right_auts, ), ]

I think this is because once you add any multi-line in an expression, the plugin expects everything to be multi-line, and also the fact that it is a comprehension messes it a bit.

I suggest to change the splitting to right before the for, and then the plugin will not complain:

candidate_auts = [ np.concatenate((left, right)) for left, right in zip(candidate_left_auts, candidate_right_auts) ]

@katxiao adding the comma doesn't work, I also thought of that 😅

@csala that works, thanks!

* Add working libraries * Add addon * Remove pydocstyle * Fix unrelated error * Make double quotes into single (#289) * Add `flake8-eradicate` (#293) * Add addon * Remove paranthesis because the addon thinks they are code * Add addon * Remove paranthesis because the addon thinks they are code * Add `flake8-builtins` (#296) * Add addon * Update code according to addon * Add addon * Update code according to addon * Add `pandas-vet` (#299) * Add addon * Update code according to addon * Add addon * Update code according to addon * Add absolute import * Add sfs (#305) * Add `flake8-multiline-containers` (#290) * Not sure what to do about these * Add noqa's where necessary * Not sure what to do about these * Add noqa's where necessary * Address feedback * Add `flake8-expression-complexity` (#291) * Add addon * Change the complexity limit to 8 from 7 * Add addon * Change the complexity limit to 8 from 7 * Fix complexity * Rename + remove unnecessary argsort * Add `flake8-print` (#292) * Add addon * Address feedback * Add `flake8-comprehensions` (#295) * Add addon * Fix code according to addon * Add addon * Fix code according to addon * Fix errors * Fix set as literal * Address feedback * Add `pytest-style` (#297) * Add addon * Update code according to addon * Add addon * Update code according to addon * Fix errors * Delete pytest * Address feedback * Add `flake8-docstrings` (#301) * Add addon * Update code according to addon * Add addon * Update code according to addon * Update code according to pydocstyle * Fixes * Fix errors * Update docstrings * Fix typos * Add addon and ignore erros (#304) * Add `flake8-dlint` (#300) * Add addon * Ignore error * Add addon * Ignore error * Add `pep8-naming` (#298) * Add addon * Ignore all the variable names * Add addon * Ignore all the variable names * Move ignores to setup.cfg * Add `flake8-fixme` (#294) * Add addon * Remove fixme * Add addon * Remove fixme * Fix bug * Fix details

Not sure what to do about these

478e612

Add noqa's where necessary

32a211f

fealho changed the base branch from issue-278-base to master November 17, 2021 13:15

fealho changed the base branch from master to issue-278-base November 17, 2021 13:15

fealho added 3 commits November 17, 2021 05:16

Not sure what to do about these

2025451

Add noqa's where necessary

88434ef

Merge branch 'issue-278-multiline' of https://github.com/sdv-dev/copulas

65aec1e

into issue-278-multiline

fealho commented Nov 17, 2021

View reviewed changes

fealho marked this pull request as ready for review November 17, 2021 15:32

fealho requested a review from a team as a code owner November 17, 2021 15:32

fealho requested review from katxiao and pvk-developer and removed request for a team November 17, 2021 15:32

Address feedback

018140b

fealho requested a review from csala November 22, 2021 10:41

pvk-developer approved these changes Nov 22, 2021

View reviewed changes

csala approved these changes Nov 22, 2021

View reviewed changes

Merge branch 'issue-278-base' into issue-278-multiline

9a0b449

fealho merged commit 3809870 into issue-278-base Nov 23, 2021

fealho deleted the issue-278-multiline branch November 23, 2021 01:17

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add `flake8-multiline-containers` #290

Add `flake8-multiline-containers` #290

fealho commented Nov 10, 2021

codecov-commenter commented Nov 10, 2021 •

edited

Loading

fealho Nov 17, 2021

katxiao Nov 18, 2021

csala Nov 19, 2021

fealho Nov 22, 2021

fealho Nov 17, 2021

fealho Nov 17, 2021

fealho Nov 17, 2021

fealho Nov 17, 2021

katxiao Nov 18, 2021

csala Nov 19, 2021

fealho Nov 22, 2021 •

edited

Loading

Add flake8-multiline-containers #290

Add flake8-multiline-containers #290

Conversation

fealho commented Nov 10, 2021

codecov-commenter commented Nov 10, 2021 • edited Loading

Codecov Report

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

fealho Nov 22, 2021 • edited Loading

Choose a reason for hiding this comment

Add `flake8-multiline-containers` #290

Add `flake8-multiline-containers` #290

codecov-commenter commented Nov 10, 2021 •

edited

Loading

fealho Nov 22, 2021 •

edited

Loading