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

Switch to using sphinx keep-going flag #2719

Merged
merged 3 commits into from
Jun 5, 2022
Merged

Conversation

akaszynski
Copy link
Member

This PR changes our documentation build to use --keep-going rather than our custom error catching script. See sphinx-build options.

Within the PyAnsys project, we've discovered that it's a brittle script and it's better to use a tried and true method rather than potentially letting warnings slip by. Plus, it's one less script to maintain.

@github-actions github-actions bot added documentation Anything related to the documentation/website maintenance Low-impact maintenance activity labels May 30, 2022
@codecov
Copy link

codecov bot commented May 30, 2022

Codecov Report

Merging #2719 (33662fe) into main (feb3664) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #2719   +/-   ##
=======================================
  Coverage   93.81%   93.81%           
=======================================
  Files          76       76           
  Lines       16195    16195           
=======================================
  Hits        15194    15194           
  Misses       1001     1001           

tkoyama010
tkoyama010 previously approved these changes May 30, 2022
Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

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

@tkoyama010
Copy link
Member

Recommending merge on 1 June 2022 unless further comments are given or issues are raised.

@akaszynski
Copy link
Member Author

it's a brittle script

 File "<string>", line 5, in <module>
  File "/home/runner/work/pyvista/pyvista/pyvista/plotting/helpers.py", line 241, in plot
    plotter.add_mesh(var_item, **kwargs)
  File "/home/runner/work/pyvista/pyvista/pyvista/plotting/plotting.py", line 2224, in add_mesh
    mesh, scalars = prepare_smooth_shading(
  File "/home/runner/work/pyvista/pyvista/pyvista/plotting/_plotting.py", line 88, in prepare_smooth_shading
    mesh.point_data[indices_array] = np.arange(arr_sz, dtype=np.int32)
  File "/home/runner/work/pyvista/pyvista/pyvista/core/datasetattributes.py", line 227, in __setitem__
    self.set_array(value, name=key)
  File "/home/runner/work/pyvista/pyvista/pyvista/core/datasetattributes.py", line 613, in set_array
    vtk_arr = self._prepare_array(data, name, deep_copy)
  File "/home/runner/work/pyvista/pyvista/pyvista/core/datasetattributes.py", line 769, in _prepare_array
    raise ValueError(f'data length of ({data.shape[0]}) != required length ({array_len})')
ValueError: data length of (12) != required length (20)

Turns out we're already not capturing these warnings.

@akaszynski akaszynski dismissed tkoyama010’s stale review May 30, 2022 23:31

non-captured warnings

@akaszynski
Copy link
Member Author

Main issue here is somehow the split_sharp_edges setting is persisting despite restoring the cache. I've fixed that by simply adding to the split_sharp_edges example.

Since the underlying issue will be fixed anyway in #2695, I think this is sufficient for now to clean up these errors.

Copy link
Member

@tkoyama010 tkoyama010 left a comment

Choose a reason for hiding this comment

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

Approving after checking the CI pass.

@tkoyama010
Copy link
Member

Suggest merging on June 5th.

@akaszynski akaszynski merged commit 904b986 into main Jun 5, 2022
@akaszynski akaszynski deleted the ci/sphinx_keep_going branch June 5, 2022 05:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Anything related to the documentation/website maintenance Low-impact maintenance activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants