Pillow icon indicating copy to clipboard operation
Pillow copied to clipboard

Add --bugreport argument to __main__.py to omit supported formats

Open nulano opened this issue 1 year ago • 2 comments

Changes proposed in this pull request:

  • #3870 added pilinfo to help in diagnosing issues. However, because it prints a list of all supported formats, I have sometimes found it difficult to explain which lines can be relevant when asking users to provide its output, e.g. in https://github.com/python-pillow/Pillow/issues/7747#issuecomment-1908063395. Adding a --bugreport option (feel free to suggest other names) to suppress printing the list of supported formats would make it easier to explain (and should be fine for older versions which will just ignore the new option).

  • Add sys.executable, sys.prefix, sys.base_prefix to pilinfo. This can help resolve the occasional confusion when there are multiple Python versions / virtual environments involved, or highlight when Anaconda Python is used.

    For example:

    -------------------------------------------------------------------
    Pillow 10.3.0.dev0
    Python 3.11.8 (v3.11.8:db85d51d3e, Feb  6 2024, 18:02:37) [Clang 13.0.0 (clang-1300.0.29.30)]
    --------------------------------------------------------------------
    Python executable is /Library/Frameworks/Python.framework/Versions/3.11/bin/python3
    System Python files loaded from /Library/Frameworks/Python.framework/Versions/3.11
    --------------------------------------------------------------------
    Python Pillow modules loaded from /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/PIL
    Binary Pillow modules loaded from /Library/Frameworks/Python.framework/Versions/3.11/lib/python3.11/site-packages/PIL
    --------------------------------------------------------------------
    
  • Add the command to the issue template -- perhaps this change should wait until after the next release.

nulano avatar Feb 20 '24 20:02 nulano

I've seen other projects have some simple command to run to print out some handy debug info for bug reports, and have been meaning to open a PR for it but never got round to it, so thank you for this!

I agree a short command would be good rather than asking to import something.

A very common issue we get is people having multiple Python interpreters, and different Pillow versions for each (or only Pillow for one), like https://snarky.ca/why-you-should-use-python-m-pip/

So having a command they can run could mean they use the same python or python3 or py or whatever they're using for their script. But again, maybe they copy/paste python3 and it's not. However, the interpreter name in the output so that could help.

hugovk avatar Feb 20 '24 22:02 hugovk

Nice! Also a missing --bugreport would be useful as an indicator of a version older than 10.3. 🤔

aclark4life avatar Feb 20 '24 22:02 aclark4life

Naming things bike shed corner:

I was going to suggest --report over --bugreport, because it's shorter and could be more generally useful.

I've seen other projects have some simple command to run to print out some handy debug info for bug reports...

Here's what some projects use:

pipenv --support
brew doctor
scoop checkup
hatch self report  # coming soon: https://hatch.pypa.io/dev/how-to/meta/report-issues/

I think either --report or --support, what do you think?

hugovk avatar Mar 04 '24 14:03 hugovk

And I just remembered the original one I was thinking of, pip debug. For example:

Details
❯ pip debug
WARNING: This command is only meant for debugging. Do not use this with automation for parsing and getting these details, since the output and options of this command may change without notice.
pip version: pip 24.0 from /Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/pip (python 3.12)
sys.version: 3.12.2 (v3.12.2:6abddd9f6a, Feb  6 2024, 17:02:06) [Clang 13.0.0 (clang-1300.0.29.30)]
sys.executable: /Library/Frameworks/Python.framework/Versions/3.12/bin/python3
sys.getdefaultencoding: utf-8
sys.getfilesystemencoding: utf-8
locale.getpreferredencoding: UTF-8
sys.platform: darwin
sys.implementation:
  name: cpython
'cert' config value: Not specified
REQUESTS_CA_BUNDLE: None
CURL_CA_BUNDLE: None
pip._vendor.certifi.where(): /Library/Frameworks/Python.framework/Versions/3.12/lib/python3.12/site-packages/pip/_vendor/certifi/cacert.pem
pip._vendor.DEBUNDLED: False
vendored library versions:
  CacheControl==0.13.1
  colorama==0.4.6
  distlib==0.3.8
  distro==1.8.0
  msgpack==1.0.5
  packaging==21.3
  platformdirs==3.8.1
  pyparsing==3.1.0
  pyproject-hooks==1.0.0
  requests==2.31.0
  certifi==2023.07.22
  chardet==5.1.0
  idna==3.4
  urllib3==1.26.17
  rich==13.4.2 (Unable to locate actual module version, using vendor.txt specified version)
  pygments==2.15.1
  typing_extensions==4.7.1 (Unable to locate actual module version, using vendor.txt specified version)
  resolvelib==1.0.1
  setuptools==68.0.0 (Unable to locate actual module version, using vendor.txt specified version)
  six==1.16.0
  tenacity==8.2.2 (Unable to locate actual module version, using vendor.txt specified version)
  tomli==2.0.1
  truststore==0.8.0
  webencodings==0.5.1 (Unable to locate actual module version, using vendor.txt specified version)
Compatible tags: 582
  cp312-cp312-macosx_14_0_arm64
  cp312-cp312-macosx_14_0_universal2
  cp312-cp312-macosx_13_0_arm64
  cp312-cp312-macosx_13_0_universal2
  cp312-cp312-macosx_12_0_arm64
  cp312-cp312-macosx_12_0_universal2
  cp312-cp312-macosx_11_0_arm64
  cp312-cp312-macosx_11_0_universal2
  cp312-cp312-macosx_10_16_universal2
  cp312-cp312-macosx_10_15_universal2
  ...
  [First 10 tags shown. Pass --verbose to show all.]

So we can throw debug into the bikeshed.

Here was a cut down version of their debug output I'd started on last October, but not got very far with:

Details
import importlib.metadata
import locale
import os
import platform
import sys
from typing import Any


def show_value(name: str, value: Any) -> None:
    print(f"{name}:\t{value}")


def get_pip_version() -> str:
    pip_pkg_dir = os.path.join(os.path.dirname(__file__), "..", "..")
    pip_pkg_dir = os.path.abspath(pip_pkg_dir)

    __version__ = importlib.metadata.version("pip")

    return f"pip {__version__} from {pip_pkg_dir}"


def debug_info() -> None:
    # print(f"sys.version\n\t{sys.version}\n")
    # print(f"platform.platform()\n\t{platform.platform()}\n")
    # print(f"platform.version()\n\t{platform.version()}\n")
    # print(f"sys.implementation.name\n\t{sys.implementation.name}\n")

    show_value("pip version", get_pip_version())
    show_value("sys.version", sys.version)
    show_value("sys.executable", sys.executable)
    show_value("sys.getdefaultencoding", sys.getdefaultencoding())
    show_value("sys.getfilesystemencoding", sys.getfilesystemencoding())
    show_value(
        "locale.getpreferredencoding",
        locale.getpreferredencoding(),
    )
    show_value("sys.platform", sys.platform)
    show_value("sys.implementation.name", sys.implementation.name)

    show_value("platform.platform", platform.platform())
    show_value("platform.version", platform.version())

    show_value("sys.maxsize", sys.maxsize)
    show_value("64 bit", sys.maxsize > 2**32)


debug_info()

I think we can stick with what we have for now and consider adding new output later.

hugovk avatar Mar 04 '24 14:03 hugovk

I think either --report or --support, what do you think?

I'm currently leaning towards --report.

Out of context I would prefer --support, but I think that could be confusing given that it is setting supported_formats=False.

nulano avatar Mar 04 '24 18:03 nulano

Add the command to the issue template -- perhaps this change should wait until after the next release.

python3 -m PIL --report still runs successfully without this PR, just without the new information and truncated output, so I'm not really concerned about users executing it before 10.3.0.

radarhere avatar Mar 09 '24 02:03 radarhere

I think either --report or --support, what do you think?

I'm currently leaning towards --report.

Out of context I would prefer --support, but I think that could be confusing given that it is setting supported_formats=False.

I don't necessarily think that it's setting supported_formats=False is something we need to worry about, I see that more as an internal detail, and we're exposing a command for users to run.

I'm fine with either.


I was thinking, could we make this even easier: instead of python3 -m PIL --report, provide python3 -m PIL report?

But we can go another step further! Revert the changes to src/PIL/__main__.py and create src/PIL/report.py (or 🚲🎨 support.py):

from __future__ import annotations

from .features import pilinfo

pilinfo(supported_formats=False)

And then:

$ python3 -m PIL --report  # Not this

$ python -m PIL.report     # Do this

And it's an even better improvement for the REPL/a script:

from PIL import features                   # Not this
features.pilinfo(supported_formats=False)  # Not this

from PIL import report                     # Do this

hugovk avatar Mar 09 '24 13:03 hugovk

python -m PIL.report

The issue with this is that it would only work for new versions, whereas adding an argument to __main__ is backwards compatible (in that the argument is ignored on old versions). That said, we can still add it and switch to asking for python -m PIL.report later.

nulano avatar Mar 09 '24 13:03 nulano

python -m PIL.report

The issue with this is that it would only work for new versions, whereas adding an argument to __main__ is backwards compatible (in that the argument is ignored on old versions).

That's fine :) We only support the latest version anyway. If someone reports an issue for an old version, a common question is "does it work with the latest version?"

That said, we can still add it and switch to asking for python -m PIL.report later.

After how long could we switch? Would we consider --report part of the public API and need deprecating --report for a year+ before removal, or could we remove that it warning? (I'd lean to the latter.)

I think it'd be easier just to have python -m PIL.report now instead of python -m PIL --report.

We can always ask people to run python -m PIL to get the full output if for some reason we need it for an old version.

hugovk avatar Mar 09 '24 15:03 hugovk

python -m PIL.report

The issue with this is that it would only work for new versions, whereas adding an argument to __main__ is backwards compatible (in that the argument is ignored on old versions).

That's fine :) We only support the latest version anyway. If someone reports an issue for an old version, a common question is "does it work with the latest version?"

That said, we can still add it and switch to asking for python -m PIL.report later.

After how long could we switch? Would we consider --report part of the public API and need deprecating --report for a year+ before removal, or could we remove that it warning? (I'd lean to the latter.)

I think it'd be easier just to have python -m PIL.report now instead of python -m PIL --report.

We can always ask people to run python -m PIL to get the full output if for some reason we need it for an old version.

@radarhere Do you have an opinion on this?

nulano avatar Mar 19 '24 17:03 nulano

Taking a quick look through closed issues, most users do file reports using the latest version of Pillow - but not all. On March 1 for example, there was an issue from a user with Pillow 8.4.0. However, not all users follow our issue template either.

We ask users in the issue template what version of Pillow they're running. That seems odd if you can't follow the instructions in the issue template for anything except the latest version.

So i guess I lean towards the backwards compatible version.

radarhere avatar Mar 20 '24 05:03 radarhere

Taking a quick look through closed issues, most users do file reports using the latest version of Pillow - but not all. On March 1 for example, there was an issue from a user with Pillow 8.4.0.

For that issue, we asked if they could retest with the latest version (they didn't answer) and for more info with a feature check (they didn't answer), and it was auto-closed due to lack of feedback.

Anyway, in the template, shall we put this:

Please paste here the output of running:

python3 -m PIL.report
or
python3 -m PIL --report

Or the output of the following Python code:

from PIL import report
# or
from PIL import features
features.pilinfo(supported_formats=False)

Then after a couple of releases or so, just have the new one?

hugovk avatar Mar 21 '24 16:03 hugovk

I think test_report.py can be combined into test_main.py. I've created https://github.com/nulano/Pillow/pull/34

radarhere avatar Mar 30 '24 07:03 radarhere

Thank you!

hugovk avatar Mar 30 '24 09:03 hugovk