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

Re-applied Black formatting to tardis after excluding undesired directories #1213

Merged
merged 4 commits into from
Jul 29, 2020

Conversation

jaladh-singhal
Copy link
Member

This PR aims to exclude undesired directories from black formatting. When running black, there are still 10 files which were formatted blacked (which were not covered earlier run, because it was run not on root of project but inside tardis/).

A brief summary of changes in each file are:

  • pyproject.toml - Added directories to exclude from black (including astropy_helpers which was being formatted by black)
  • tardis_env3.yml - Added black as a code quality dependency, so that it is already installed in a new tardis environment
  • 10 files formatted by black are distributed as follows:
    • tardis/plasma/ - 2 files - they are actually one line changes, made by us @wkerzendorf when editing strings - those strings were quite long, so black line-breaked them as expected ✅
    • docs/ - There are 4 python files inside it
    • asv/ - 1 file
    • ah_bootstrap.py, setup.py, ez_setup.py are remaining 3 at root directory.

Motivation and Context

Related #1201

How Has This Been Tested?

They're only formatting changes so ideally they shouldn't break the code. And if test pipeline pass, they are good to merge.

@codecov
Copy link

codecov bot commented Jun 30, 2020

Codecov Report

Merging #1213 into master will not change coverage.
The diff coverage is 50.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1213   +/-   ##
=======================================
  Coverage   77.71%   77.71%           
=======================================
  Files          91       91           
  Lines        5727     5727           
=======================================
  Hits         4451     4451           
  Misses       1276     1276           
Impacted Files Coverage Δ
tardis/plasma/standard_plasmas.py 83.95% <0.00%> (ø)
tardis/plasma/properties/general.py 98.38% <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 f47075d...a2de4c4. Read the comment docs.

Copy link
Member Author

@jaladh-singhal jaladh-singhal left a comment

Choose a reason for hiding this comment

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

@wkerzendorf On reviewing each file, these are the changes I think we'll require - otherwise everything looks good to me! Please if you can, take a moment to see anything else you may find!

And if these changes seem fine by you, let me know - I'll do a batch commit.

ah_bootstrap.py Outdated Show resolved Hide resolved
docs/conf.py Outdated Show resolved Hide resolved
ez_setup.py Outdated Show resolved Hide resolved
ez_setup.py Outdated Show resolved Hide resolved
ez_setup.py Outdated Show resolved Hide resolved
ez_setup.py Outdated Show resolved Hide resolved
@jaladh-singhal
Copy link
Member Author

Excluded ah_bootstrap.py, setup.py, ez_setup.py and docs/conf.py, rest 6 files formatted by black looks good to me!

@wkerzendorf wkerzendorf requested a review from harpolea July 8, 2020 13:54
@wkerzendorf wkerzendorf merged commit 3787589 into tardis-sn:master Jul 29, 2020
atharva-2001 pushed a commit to atharva-2001/tardis that referenced this pull request Oct 1, 2021
…tories (tardis-sn#1213)

* Added directories to exclude in black configuration file

* Added black to tardis environment file

* Excluded setup related files from black configuration

* Applied black formatting to remaining files
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.

3 participants