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

Sofia_newfeature #255

Merged
merged 7 commits into from Jul 1, 2019
Merged

Sofia_newfeature #255

merged 7 commits into from Jul 1, 2019

Conversation

OVSofia
Copy link
Member

@OVSofia OVSofia commented Jun 25, 2019

Fixes #243 #244 #245

@pep8speaks
Copy link

pep8speaks commented Jun 25, 2019

Hello @OVSofia! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 16:9: W503 line break before binary operator
Line 22:13: W503 line break before binary operator
Line 24:9: W503 line break before binary operator

Comment last updated at 2019-07-01 05:34:44 UTC

@shreyasbapat
Copy link
Member

You will have to run:

tox -e reformat

when in einsteinpy folder :)

src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Show resolved Hide resolved
.gitignore Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Outdated Show resolved Hide resolved
@ritzvik
Copy link
Member

ritzvik commented Jun 26, 2019

Hi @OVSofia , thanks for your efforts.
Left some comments.

Also, your branch has diverged too much from the upstream, but don't take pains now. Work on this PR, we would help you fixing the git problem once this PR is final :)

@ritzvik
Copy link
Member

ritzvik commented Jun 26, 2019

Fixes #<Put_Issue_Number_Here>

You might want to put the issue numbers this PR is fixing.

@OVSofia
Copy link
Member Author

OVSofia commented Jun 26, 2019

You will have to run:

tox -e reformat

when in einsteinpy folder :)

OH no! :O
I had forgotten

@ritzvik
Copy link
Member

ritzvik commented Jun 27, 2019

@OVSofia kindly resolve the comments which you think you have sorted out

src/einsteinpy/symbolic/metric.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/tests/test_symbolic/test_ricci.py Outdated Show resolved Hide resolved
src/einsteinpy/tests/test_symbolic/test_ricci.py Outdated Show resolved Hide resolved
@ritzvik
Copy link
Member

ritzvik commented Jun 28, 2019

@OVSofia you can run tox -e reformat or

  • black ./ and isort -rc ./ while in the src/einsteinpy directory.

@ritzvik
Copy link
Member

ritzvik commented Jun 28, 2019

@OVSofia , you can always check whether your tests are working or not on your system.
pytest -vv

or pytest -vv /path/to/file/or/directory for running specific test file or folder.

@ritzvik
Copy link
Member

ritzvik commented Jun 29, 2019

Also, @OVSofia could you remove the __setitem__ function from tensor.py as sympy arrays are immutable, calling it would raise TypeError

That was my bad as I didn't see to this earlier.

src/einsteinpy/symbolic/ricci.py Show resolved Hide resolved
src/einsteinpy/symbolic/ricci.py Show resolved Hide resolved
src/einsteinpy/symbolic/tensor.py Outdated Show resolved Hide resolved
src/einsteinpy/symbolic/vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/tests/test_symbolic/test_vaccum_metrics.py Outdated Show resolved Hide resolved
src/einsteinpy/tests/test_symbolic/test_vaccum_metrics.py Outdated Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jun 30, 2019

Codecov Report

Merging #255 into master will increase coverage by 0.08%.
The diff coverage is 97.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   94.14%   94.22%   +0.08%     
==========================================
  Files          28       29       +1     
  Lines        1024     1057      +33     
==========================================
+ Hits          964      996      +32     
- Misses         60       61       +1
Impacted Files Coverage Δ
src/einsteinpy/symbolic/vacuum_metrics.py 100% <100%> (ø)
src/einsteinpy/symbolic/tensor.py 100% <100%> (ø) ⬆️
src/einsteinpy/symbolic/ricci.py 95.65% <95.23%> (-4.35%) ⬇️

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 e869ea7...1ac1139. Read the comment docs.

1 similar comment
@codecov
Copy link

codecov bot commented Jun 30, 2019

Codecov Report

Merging #255 into master will increase coverage by 0.08%.
The diff coverage is 97.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   94.14%   94.22%   +0.08%     
==========================================
  Files          28       29       +1     
  Lines        1024     1057      +33     
==========================================
+ Hits          964      996      +32     
- Misses         60       61       +1
Impacted Files Coverage Δ
src/einsteinpy/symbolic/vacuum_metrics.py 100% <100%> (ø)
src/einsteinpy/symbolic/tensor.py 100% <100%> (ø) ⬆️
src/einsteinpy/symbolic/ricci.py 95.65% <95.23%> (-4.35%) ⬇️

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 e869ea7...1ac1139. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 30, 2019

Codecov Report

Merging #255 into master will increase coverage by 0.09%.
The diff coverage is 97.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   93.64%   93.74%   +0.09%     
==========================================
  Files          30       31       +1     
  Lines        1102     1135      +33     
==========================================
+ Hits         1032     1064      +32     
- Misses         70       71       +1
Impacted Files Coverage Δ
src/einsteinpy/symbolic/vacuum_metrics.py 100% <100%> (ø)
src/einsteinpy/symbolic/tensor.py 100% <100%> (ø) ⬆️
src/einsteinpy/symbolic/ricci.py 95.65% <95.23%> (-4.35%) ⬇️

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 8ccce0c...f76099b. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 30, 2019

Codecov Report

Merging #255 into master will increase coverage by 0.08%.
The diff coverage is 97.05%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #255      +/-   ##
==========================================
+ Coverage   94.14%   94.22%   +0.08%     
==========================================
  Files          28       29       +1     
  Lines        1024     1057      +33     
==========================================
+ Hits          964      996      +32     
- Misses         60       61       +1
Impacted Files Coverage Δ
src/einsteinpy/symbolic/vacuum_metrics.py 100% <100%> (ø)
src/einsteinpy/symbolic/tensor.py 100% <100%> (ø) ⬆️
src/einsteinpy/symbolic/ricci.py 95.65% <95.23%> (-4.35%) ⬇️

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 e869ea7...1ac1139. Read the comment docs.

@ritzvik
Copy link
Member

ritzvik commented Jul 1, 2019

@OVSofia, see all the tests have passed now.
You have to now rebase your branch.

git pull --rebase upstream master
git push origin sof_newfeature -f

This is would be good for merging then.

@shreyasbapat
Copy link
Member

Don't add labels!

@shreyasbapat
Copy link
Member

@OVSofia I have edited your Pull request description! Please have a look and see how I linked the respective issues!
Please make sure that from the next time, you also do the same!

@shreyasbapat
Copy link
Member

shreyasbapat commented Jul 1, 2019

I was waiting for this pull request to complete. Otherwise we would not have had any new features from SOCIS student in the release!
Now after @Varunvaruns9 makes his Pull request, Feature Freeze will begin!

@shreyasbapat
Copy link
Member

I am merging this!

@shreyasbapat shreyasbapat merged commit 54f25e0 into einsteinpy:master Jul 1, 2019
@ritzvik
Copy link
Member

ritzvik commented Jul 1, 2019

Both @OVSofia and I forgot to add the docs for Ricci Tensor and Vacuum metrics.
@shreyasbapat Kindly do this with any of your PRs

@OVSofia
Copy link
Member Author

OVSofia commented Jul 1, 2019

I was waiting for this pull request to complete. Otherwise we would not have had any new features from SOCIS student in the release!
Now after @Varunvaruns9 makes his Pull request, Feature Freeze will begin!

Ohh okey! And how long will it last? Can I make push when the Feature Freeze is underway?

@ritzvik
Copy link
Member

ritzvik commented Jul 1, 2019

I was waiting for this pull request to complete. Otherwise we would not have had any new features from SOCIS student in the release!
Now after @Varunvaruns9 makes his Pull request, Feature Freeze will begin!

Ohh okey! And how long will it last? Can I make push when the Feature Freeze is underway?

You can make PRs as you wish. It would just not be merged!
BTW see #267

@shreyasbapat
Copy link
Member

They will be merged but in master! Not in 0.2.x branch!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement Ricci Tensor symbolically
4 participants