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

Simplify "all" optional dependency, add pytest_report_header #4407

Merged
merged 1 commit into from
May 13, 2023

Conversation

mwtoews
Copy link
Contributor

@mwtoews mwtoews commented May 10, 2023

Overview

  • Simplify the "all" optional dependency in pyproject.toml to remove repetitions
  • Implement a pytest report header that shows the required and optional packages and their versions. Also a report of what packages are not installed.

Details

When running pytest, the top of the report might look something like:

============================= test session starts =============================
platform win32 -- Python 3.8.10, pytest-7.3.1, pluggy-1.0.0 -- C:\hostedtoolcache\windows\Python\3.8.10\x64\python.exe
cachedir: .pytest_cache
hypothesis profile 'default' -> database=DirectoryBasedExampleDatabase('D:\\a\\pyvista\\pyvista\\.hypothesis\\examples')
required packages: matplotlib-3.7.1, numpy-1.24.3, pillow-9.5.0, pooch-1.7.0, scooby-0.7.2, vtk-9.2.6
optional 'colormaps' packages: cmocean-3.0.3, colorcet-3.0.1
optional 'io' packages: imageio-2.28.1, meshio-5.3.4
optional 'jupyter' packages: ipyvtklink-0.2.3, ipywidgets-7.7.5, nest_asyncio-1.5.6, panel-0.14.4, pythreejs-2.4.2, trame-2.4.0, trame-client-2.7.5, trame-server-2.11.0, trame-vtk-2.4.4
optional 'trame' packages: trame-2.4.0, trame-client-2.7.5, trame-server-2.11.0, trame-vtk-2.4.4
optional package not found: jupyter-server-proxy
rootdir: D:\a\pyvista\pyvista
configfile: pyproject.toml
testpaths: tests
plugins: anyio-3.6.2, hypothesis-6.75.2, cov-4.0.0, memprof-0.2.0, pytest_pyvista-0.1.8, xdist-3.2.1
collecting ... collected [19](https://github.com/pyvista/pyvista/actions/runs/4942108414/jobs/8835357232#step:10:20)58 items

@github-actions github-actions bot added the maintenance Low-impact maintenance activity label May 10, 2023
@codecov
Copy link

codecov bot commented May 10, 2023

Codecov Report

Merging #4407 (d86330b) into main (461bb5e) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main    #4407   +/-   ##
=======================================
  Coverage   95.82%   95.82%           
=======================================
  Files          97       97           
  Lines       20874    20874           
=======================================
  Hits        20002    20002           
  Misses        872      872           

'trame-vtk>=2.4.0',
'trame>=2.2.6',
]
all = ['pyvista[colormaps,io,jupyter,trame]']
Copy link
Member

@tkoyama010 tkoyama010 May 11, 2023

Choose a reason for hiding this comment

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

This expression is nice. Could you show us where in the Python documentation you have referred to so that someone who sees this pull request later can find it? (I couldn't find it.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This appears to be a pip 21.2 support feature, described here (although I can't see it in NEWS.rst). However, I can't seem to find any reputable source.

Copy link
Member

Choose a reason for hiding this comment

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

This is great! I really like how it simplifies that our requirements.

I'm a little worried about the minimum supported version of pip, but it seems that it still installs, it just doesn't respect the extras:

(py38) $ which python
/home/alex/python/py38/bin/python
(py38) $ pip -V
pip 19.1 from /home/alex/python/py38/lib/python3.8/site-packages/pip (python 3.8)
(py38) $ pip install .[all]
Processing /home/alex/source/pyvista
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Installing backend dependencies ... done
    Preparing wheel metadata ... done
Requirement already satisfied: scooby>=0.5.1 in /home/alex/python/scooby (from pyvista==0.40.dev0) (0.7.dev0)
Requirement already satisfied: pooch in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (1.6.0.post14+gc77fd01)
Requirement already satisfied: numpy in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (1.22.3)
Requirement already satisfied: pillow in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (9.1.0)
Requirement already satisfied: matplotlib>=3.0.1 in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (3.0.1)
Requirement already satisfied: vtk in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (9.2.2)
Requirement already satisfied: appdirs>=1.3.0 in /home/alex/python/py38/lib/python3.8/site-packages (from pooch->pyvista==0.40.dev0) (1.4.4)
Requirement already satisfied: packaging>=20.0 in /home/alex/python/py38/lib/python3.8/site-packages (from pooch->pyvista==0.40.dev0) (21.3)
Requirement already satisfied: requests>=2.19.0 in /home/alex/python/py38/lib/python3.8/site-packages (from pooch->pyvista==0.40.dev0) (2.28.1)
Requirement already satisfied: kiwisolver>=1.0.1 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (1.4.2)
Requirement already satisfied: python-dateutil>=2.1 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (2.8.2)
Requirement already satisfied: cycler>=0.10 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (0.11.0)
Requirement already satisfied: pyparsing!=2.0.4,!=2.1.2,!=2.1.6,>=2.0.1 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (3.0.8)
Requirement already satisfied: wslink>=1.0.4 in /home/alex/python/py38/lib/python3.8/site-packages (from vtk->pyvista==0.40.dev0) (1.10.1)
Requirement already satisfied: idna<4,>=2.5 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (3.3)
Requirement already satisfied: urllib3<1.27,>=1.21.1 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (1.26.9)
Requirement already satisfied: certifi>=2017.4.17 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (2021.10.8)
Requirement already satisfied: charset-normalizer<3,>=2 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (2.0.12)
Requirement already satisfied: six>=1.5 in /home/alex/python/py38/lib/python3.8/site-packages (from python-dateutil>=2.1->matplotlib>=3.0.1->pyvista==0.40.dev0) (1.16.0)
Requirement already satisfied: aiohttp<4 in /home/alex/python/py38/lib/python3.8/site-packages (from wslink>=1.0.4->vtk->pyvista==0.40.dev0) (3.8.1)
Requirement already satisfied: attrs>=17.3.0 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (21.4.0)
Requirement already satisfied: multidict<7.0,>=4.5 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (6.0.2)
Requirement already satisfied: async-timeout<5.0,>=4.0.0a3 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (4.0.2)
Requirement already satisfied: frozenlist>=1.1.1 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (1.3.0)
Requirement already satisfied: yarl<2.0,>=1.0 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (1.7.2)
Requirement already satisfied: aiosignal>=1.1.2 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (1.2.0)
Building wheels for collected packages: pyvista
  Building wheel for pyvista (PEP 517) ... done
  Stored in directory: /tmp/pip-ephem-wheel-cache-w67jgp03/wheels/e2/2c/5c/bc1c764f32904c3e089b190dd26847c33b3a3427ece0910930
Successfully built pyvista
Installing collected packages: pyvista
  Found existing installation: pyvista 0.40.dev0
    Uninstalling pyvista-0.40.dev0:
      Successfully uninstalled pyvista-0.40.dev0
Successfully installed pyvista-0.40.dev0

Since pip is pretty annoying about upgrading and it doesn't break the install, I view this as worth it. By the time that v0.40.0 is out, We will have dropped Python 3.7, putting us that much closer to being able to support this on fresh installs with base Python.

I still could go either way on this, but I'm leaning in the direction of including it.

Copy link
Contributor

Choose a reason for hiding this comment

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

By the time that v0.40.0 is out, We will have dropped Python 3.7, putting us that much closer to being able to support this on fresh installs with base Python.

Isn't this already true for v0.39.0? Given that you've suggested that this is tied to Python version, I checked classifiers for pip and v19.3 is the last version that does not claim to support v3.8. Confusingly, pip v19.3.1 does. https://pypi.org/project/pip/19.3.1/. So it is quite possible that several older versions of pip are also compatible with v3.8 and pip does not pin an upper python version. Users could be using pip <19.3 and python3.8 potentially.

Having said all this, is there a big downside to removing support for very old pip versions, especially given that most of the options require fairly up to date dependencies? If not, I would lean the same way.

Copy link
Member

Choose a reason for hiding this comment

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

Having said all this, is there a big downside to removing support for very old pip versions, especially given that most of the options require fairly up to date dependencies? If not, I would lean the same way.

Considering that this still works even with an older version of pip (that are technically unsupported), then I think this is good to go.

Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

Looks great. Only one issue in regards to minimum pip support.

'trame-vtk>=2.4.0',
'trame>=2.2.6',
]
all = ['pyvista[colormaps,io,jupyter,trame]']
Copy link
Member

Choose a reason for hiding this comment

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

This is great! I really like how it simplifies that our requirements.

I'm a little worried about the minimum supported version of pip, but it seems that it still installs, it just doesn't respect the extras:

(py38) $ which python
/home/alex/python/py38/bin/python
(py38) $ pip -V
pip 19.1 from /home/alex/python/py38/lib/python3.8/site-packages/pip (python 3.8)
(py38) $ pip install .[all]
Processing /home/alex/source/pyvista
  Installing build dependencies ... done
  Getting requirements to build wheel ... done
  Installing backend dependencies ... done
    Preparing wheel metadata ... done
Requirement already satisfied: scooby>=0.5.1 in /home/alex/python/scooby (from pyvista==0.40.dev0) (0.7.dev0)
Requirement already satisfied: pooch in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (1.6.0.post14+gc77fd01)
Requirement already satisfied: numpy in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (1.22.3)
Requirement already satisfied: pillow in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (9.1.0)
Requirement already satisfied: matplotlib>=3.0.1 in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (3.0.1)
Requirement already satisfied: vtk in /home/alex/python/py38/lib/python3.8/site-packages (from pyvista==0.40.dev0) (9.2.2)
Requirement already satisfied: appdirs>=1.3.0 in /home/alex/python/py38/lib/python3.8/site-packages (from pooch->pyvista==0.40.dev0) (1.4.4)
Requirement already satisfied: packaging>=20.0 in /home/alex/python/py38/lib/python3.8/site-packages (from pooch->pyvista==0.40.dev0) (21.3)
Requirement already satisfied: requests>=2.19.0 in /home/alex/python/py38/lib/python3.8/site-packages (from pooch->pyvista==0.40.dev0) (2.28.1)
Requirement already satisfied: kiwisolver>=1.0.1 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (1.4.2)
Requirement already satisfied: python-dateutil>=2.1 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (2.8.2)
Requirement already satisfied: cycler>=0.10 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (0.11.0)
Requirement already satisfied: pyparsing!=2.0.4,!=2.1.2,!=2.1.6,>=2.0.1 in /home/alex/python/py38/lib/python3.8/site-packages (from matplotlib>=3.0.1->pyvista==0.40.dev0) (3.0.8)
Requirement already satisfied: wslink>=1.0.4 in /home/alex/python/py38/lib/python3.8/site-packages (from vtk->pyvista==0.40.dev0) (1.10.1)
Requirement already satisfied: idna<4,>=2.5 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (3.3)
Requirement already satisfied: urllib3<1.27,>=1.21.1 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (1.26.9)
Requirement already satisfied: certifi>=2017.4.17 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (2021.10.8)
Requirement already satisfied: charset-normalizer<3,>=2 in /home/alex/python/py38/lib/python3.8/site-packages (from requests>=2.19.0->pooch->pyvista==0.40.dev0) (2.0.12)
Requirement already satisfied: six>=1.5 in /home/alex/python/py38/lib/python3.8/site-packages (from python-dateutil>=2.1->matplotlib>=3.0.1->pyvista==0.40.dev0) (1.16.0)
Requirement already satisfied: aiohttp<4 in /home/alex/python/py38/lib/python3.8/site-packages (from wslink>=1.0.4->vtk->pyvista==0.40.dev0) (3.8.1)
Requirement already satisfied: attrs>=17.3.0 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (21.4.0)
Requirement already satisfied: multidict<7.0,>=4.5 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (6.0.2)
Requirement already satisfied: async-timeout<5.0,>=4.0.0a3 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (4.0.2)
Requirement already satisfied: frozenlist>=1.1.1 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (1.3.0)
Requirement already satisfied: yarl<2.0,>=1.0 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (1.7.2)
Requirement already satisfied: aiosignal>=1.1.2 in /home/alex/python/py38/lib/python3.8/site-packages (from aiohttp<4->wslink>=1.0.4->vtk->pyvista==0.40.dev0) (1.2.0)
Building wheels for collected packages: pyvista
  Building wheel for pyvista (PEP 517) ... done
  Stored in directory: /tmp/pip-ephem-wheel-cache-w67jgp03/wheels/e2/2c/5c/bc1c764f32904c3e089b190dd26847c33b3a3427ece0910930
Successfully built pyvista
Installing collected packages: pyvista
  Found existing installation: pyvista 0.40.dev0
    Uninstalling pyvista-0.40.dev0:
      Successfully uninstalled pyvista-0.40.dev0
Successfully installed pyvista-0.40.dev0

Since pip is pretty annoying about upgrading and it doesn't break the install, I view this as worth it. By the time that v0.40.0 is out, We will have dropped Python 3.7, putting us that much closer to being able to support this on fresh installs with base Python.

I still could go either way on this, but I'm leaning in the direction of including it.

Copy link
Member

@akaszynski akaszynski left a comment

Choose a reason for hiding this comment

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

LGTM. pip version issues are, IMO, not an issue. At worst they'll make pip install pyvista[all] just install the base library on very old versions of pip.

Copy link
Member

@banesullivan banesullivan left a comment

Choose a reason for hiding this comment

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

+1 to @akaszynski's evaluation

@banesullivan banesullivan mentioned this pull request May 12, 2023
akaszynski added a commit that referenced this pull request May 12, 2023
@akaszynski akaszynski merged commit 9f5e989 into pyvista:main May 13, 2023
@mwtoews mwtoews deleted the optional-dep-all branch May 13, 2023 18:07
tkoyama010 added a commit that referenced this pull request May 14, 2023
* convert back to polydata using extact_geometry

* improved readability

* branch for performance

* add in verts

* revert #4407 into this branch

* Avoided omission of two letters.

---------

Co-authored-by: Tetsuo Koyama <[email protected]>
akaszynski added a commit that referenced this pull request May 16, 2023
* convert back to polydata using extact_geometry

* improved readability

* branch for performance

* add in verts

* revert #4407 into this branch

* Avoided omission of two letters.

---------

Co-authored-by: Tetsuo Koyama <[email protected]>
akaszynski added a commit that referenced this pull request May 16, 2023
* Add PyHyperbolic3D as an external example (#4420)

* Add PyHyperbolic3D as an external example

* Reduce Gif file size

* Use 128 color

* optimize and scale

---------

Co-authored-by: Alex Kaszynski <[email protected]>

* Fix merge dataset (#4414)

* convert back to polydata using extact_geometry

* improved readability

* branch for performance

* add in verts

* revert #4407 into this branch

* Avoided omission of two letters.

---------

Co-authored-by: Tetsuo Koyama <[email protected]>

* Remove SKIP in docstrings (#4419)

* remove SKIPs

* remove the rest of skips

* minor fixes

* Fix active scalars and pass point data in `to_tetrahedra` for `RectilinearGrid` (#4406)

* add failing test

* fix active scalars for cell data

* pass point data, deprecate use of pass_cell_data

* document parameter

---------

Co-authored-by: Alex Kaszynski <[email protected]>

* Add gif extension to Sphinx-Gallery scraper (#4403)

* Add gif extension

* Reset cache

* Add test for GIFs

* Revert cache regeneration

* Fix test

* use cells for the example

---------

Co-authored-by: Alex Kaszynski <[email protected]>

* bump version to v0.39.1

---------

Co-authored-by: Tetsuo Koyama <[email protected]>
Co-authored-by: MatthewFlamm <[email protected]>
Co-authored-by: Alex Fernandez <[email protected]>
@banesullivan banesullivan mentioned this pull request Jun 30, 2023
6 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Low-impact maintenance activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants