Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Enabled pylint generator checks #1588

Merged
merged 5 commits into from
Mar 3, 2021

Conversation

MgeeeeK
Copy link
Contributor

@MgeeeeK MgeeeeK commented Feb 27, 2021

Description

Enabled 2 pylint checks namely:

  • use-a-generator
  • consider-using-generator

Fix #1578

Checklist

  • Follows official PR format
  • Code style correct (follows pylint and black guidelines)
  • Changes are listed in changelog

@MgeeeeK MgeeeeK changed the title Enabled pylint generator checks [WIP]: Enabled pylint generator checks Feb 27, 2021
@codecov
Copy link

codecov bot commented Feb 28, 2021

Codecov Report

Merging #1588 (36f7e81) into main (c562fad) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #1588   +/-   ##
=======================================
  Coverage   90.05%   90.05%           
=======================================
  Files         108      108           
  Lines       11584    11584           
=======================================
  Hits        10432    10432           
  Misses       1152     1152           
Impacted Files Coverage Δ
arviz/data/base.py 97.87% <100.00%> (ø)
arviz/data/io_pymc3.py 90.68% <100.00%> (ø)
arviz/plots/backends/bokeh/energyplot.py 93.44% <100.00%> (ø)
arviz/plots/backends/matplotlib/energyplot.py 90.00% <100.00%> (ø)
arviz/plots/backends/matplotlib/ppcplot.py 98.07% <100.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 c562fad...69954a0. Read the comment docs.

@MgeeeeK
Copy link
Contributor Author

MgeeeeK commented Feb 28, 2021

Hi @OriolAbril , you can take a look now

@MgeeeeK MgeeeeK changed the title [WIP]: Enabled pylint generator checks Enabled pylint generator checks Mar 1, 2021
CHANGELOG.md Outdated
@@ -13,6 +13,7 @@
* Integrate `index_origin` with all the library ([1201](https://github.com/arviz-devs/arviz/pull/1201))
* Fix pareto k threshold typo in reloo function ([1580](https://github.com/arviz-devs/arviz/pull/1580))
* Preserve shape from Stan code in `from_cmdstanpy` ([1579](https://github.com/arviz-devs/arviz/pull/1579))
* Enabled pylint `use-a-generator` and `consider-using-generator` checks ([1588](https://github.com/arviz-devs/arviz/pull/1588))
Copy link
Member

Choose a reason for hiding this comment

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

I would write something like Use generators instead of lists whenever possible, users reading the changelog and using arviz may not even know what pylint is, not sure they would understand what this change is.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, that makes sense.

@MgeeeeK MgeeeeK requested a review from OriolAbril March 3, 2021 12:33
@OriolAbril OriolAbril merged commit 43ceb17 into arviz-devs:main Mar 3, 2021
@OriolAbril
Copy link
Member

Thanks!

@MgeeeeK MgeeeeK deleted the pylint-generator-check branch March 26, 2021 07:54
utkarsh-maheshwari pushed a commit to utkarsh-maheshwari/arviz that referenced this pull request May 27, 2021
* Enabled pylint generator check

* Fixed generator related warnings raised by pylint

* Updated the changelog

* Changed the changelog msg

Co-authored-by: Oriol Abril-Pla <oriol.abril.pla@gmail.com>
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.

Enable pylint use-a-generator check
2 participants