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.

Sign up for GitHub

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

Jump to bottom

Enable Nitpicky mode #7

Merged
merged 4 commits into from
Jun 2, 2021
Merged

Enable Nitpicky mode #7

merged 4 commits into from
Jun 2, 2021

Conversation

jeffreypaul15
Copy link
Collaborator

Nitpicky stuff from #6
Does this need a changelog?

@nabobalis nabobalis added the No Changelog Entry Needed Skip any changelog checks. label Jun 1, 2021
@nabobalis
Copy link
Contributor

No its ok.

@nabobalis
Copy link
Contributor 

/home/vsts/work/1/s/.tox/build_docs/lib/python3.8/site-packages/sunkit_pyvista/plotter.py:docstring of sunkit_pyvista.plotter.SunpyPlotter:: WARNING: py:class reference target not found: astropy.coordinates.BaseFrame
/home/vsts/work/1/s/.tox/build_docs/lib/python3.8/site-packages/pyvista/plotting/plotting.py:docstring of pyvista.plotting.plotting.Plotter.show:: WARNING: py:class reference target not found: string
/home/vsts/work/1/s/.tox/build_docs/lib/python3.8/site-packages/pyvista/plotting/plotting.py:docstring of pyvista.plotting.plotting.Plotter.show:: WARNING: py:class reference target not found: floats

Looks like three left.

@jeffreypaul15
Copy link
Collaborator Author 

/home/vsts/work/1/s/.tox/build_docs/lib/python3.8/site-packages/sunkit_pyvista/plotter.py:docstring of sunkit_pyvista.plotter.SunpyPlotter:: WARNING: py:class reference target not found: astropy.coordinates.BaseFrame
/home/vsts/work/1/s/.tox/build_docs/lib/python3.8/site-packages/pyvista/plotting/plotting.py:docstring of pyvista.plotting.plotting.Plotter.show:: WARNING: py:class reference target not found: string
/home/vsts/work/1/s/.tox/build_docs/lib/python3.8/site-packages/pyvista/plotting/plotting.py:docstring of pyvista.plotting.plotting.Plotter.show:: WARNING: py:class reference target not found: floats

Looks like three left.

One for us though, the other 2 are Pyvista's docstrings right?

@nabobalis
Copy link
Contributor

True but I assume this comes from the show in our code as it wraps the pyvista one.

@jeffreypaul15
Copy link
Collaborator Author

True but I assume this comes from the show in our code as it wraps the pyvista one.

How do we fix this then?

@nabobalis
Copy link
Contributor

Not sure.

@dstansby
Copy link
Member

dstansby commented Jun 1, 2021

I would

  • Add them to nitpick ignore
  • Raise an upstream issue with pyvista to fix them

@jeffreypaul15
Copy link
Collaborator Author 

* Raise an upstream issue with `pyvista` to fix them

Pyvista's codebase is filled with these dcstring issues though

@dstansby
Copy link
Member

dstansby commented Jun 2, 2021

Okay, lets leave this as is then, and we can re-asses if we come upon more pyvista issues in the future.

@dstansby dstansby merged commit 9edbe70 into sunpy:main Jun 2, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees
No one assigned
Labels
No Changelog Entry Needed Skip any changelog checks.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@jeffreypaul15 @nabobalis @dstansby