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

Update viz.py docstrings and Inputs #193

Merged
merged 8 commits into from
Apr 2, 2020
Merged

Conversation

vinferrer
Copy link
Collaborator

Closes #188

Proposed Changes

  • Update viz.trigger_plot docstring
  • Change viz,plot_all inputs so instead of phys_in we enter all the parameters in a separated way
  • Update viz.plot_all docstring after this

@vinferrer vinferrer added Documentation This issue or PR is about the documentation Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog labels Mar 30, 2020
@vinferrer vinferrer requested a review from smoia March 30, 2020 15:18
@vinferrer vinferrer self-assigned this Mar 30, 2020
@codecov
Copy link

codecov bot commented Mar 30, 2020

Codecov Report

Merging #193 into master will not change coverage by %.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #193   +/-   ##
=======================================
  Coverage   94.42%   94.42%           
=======================================
  Files           7        7           
  Lines         574      574           
=======================================
  Hits          542      542           
  Misses         32       32           
Impacted Files Coverage Δ
phys2bids/phys2bids.py 90.44% <100.00%> (ø)
phys2bids/viz.py 97.26% <100.00%> (ø)

@codecov
Copy link

codecov bot commented Mar 30, 2020

Codecov Report

Merging #193 into master will not change coverage by %.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #193   +/-   ##
=======================================
  Coverage   94.42%   94.42%           
=======================================
  Files           7        7           
  Lines         574      574           
=======================================
  Hits          542      542           
  Misses         32       32           
Impacted Files Coverage Δ
phys2bids/viz.py 97.26% <ø> (ø)

@smoia smoia removed the Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog label Mar 30, 2020
@vinferrer vinferrer requested a review from RayStick March 31, 2020 07:57
phys2bids/viz.py Outdated Show resolved Hide resolved
RayStick
RayStick previously approved these changes Mar 31, 2020
@vinferrer
Copy link
Collaborator Author

Thank you @RayStick but i didn't finish this draft

@RayStick
Copy link
Member

RayStick commented Apr 1, 2020

Ooops, sorry, didn't notice it was still a draft!

@vinferrer vinferrer marked this pull request as ready for review April 1, 2020 15:04
@vinferrer
Copy link
Collaborator Author

Now it's ready

@RayStick RayStick dismissed their stale review April 1, 2020 15:22

Reviewed when it was a draft

phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
Copy link
Member

@RayStick RayStick left a comment

Choose a reason for hiding this comment

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

I didn't check it out locally, but looks good to me. (Just suggested a few minor wording changes)

Copy link
Member

@smoia smoia left a comment

Choose a reason for hiding this comment

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

I know writing docstrings is not as fun as writing code, but there's some fixes to do here and there before we can merge this...

phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
@vinferrer
Copy link
Collaborator Author

changes applied

@smoia smoia added the Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog label Apr 2, 2020
Copy link
Member

@smoia smoia left a comment

Choose a reason for hiding this comment

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

Couple of things but then we're good to go!

phys2bids/viz.py Show resolved Hide resolved
phys2bids/viz.py Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
phys2bids/viz.py Show resolved Hide resolved
phys2bids/viz.py Outdated Show resolved Hide resolved
Copy link
Member

@smoia smoia left a comment

Choose a reason for hiding this comment

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

LGTM!

@smoia smoia changed the title update viz.py docstrings and Inputs Update viz.py docstrings and Inputs Apr 2, 2020
@smoia smoia merged commit 0dd291d into physiopy:master Apr 2, 2020
@vinferrer vinferrer deleted the viz_docstring branch April 3, 2020 07:08
@smoia smoia added the released This issue/pull request has been released. label Oct 14, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Documentation This issue or PR is about the documentation Internal Changes affect the internal API. It doesn't increase the version, but produces a changelog released This issue/pull request has been released.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update docstrings in viz.py
3 participants