ImagingDataCommons / highdicom

High-level DICOM abstractions for the Python programming language
https://highdicom.readthedocs.io
MIT License
171 stars 37 forks source link

setup.py → pyproject.toml #293

Closed DimitriPapadopoulos closed 3 months ago

DimitriPapadopoulos commented 3 months ago

Fxies #288.

CPBridge commented 3 months ago

Thanks, looks good but I'll run a few careful tests before we merge

DimitriPapadopoulos commented 3 months ago
  1. The license is part of the classifiers:

       "License :: OSI Approved :: MIT License",

    I have not duplicated it in a license field because the Python Packaging User Guide reads:

    If you are using a standard, well-known license, it is not necessary to use this field. Instead, you should use one of the classifiers starting with License ::.

    Same for platforms, they are documented in the classifiers. There's no key dedicated to platforms anyway.

  2. I've removed requirements_doc.txt and requirements_test.txt.
CPBridge commented 3 months ago

Thanks! That makes sense.

But now the github actions are failing because they depend on the old requirements_test.txt. Could you please make the simple change to this line of run_unit_tests.yml to fix and get the CI pipeline running again?

DimitriPapadopoulos commented 3 months ago

The CI jobs pass, I have also updated the documentation.

CPBridge commented 3 months ago

I have also updated the documentation.

Ah yes, good catch. Thanks for your help!

DimitriPapadopoulos commented 3 months ago

Unfortunately, I missed an occurrence of requirements_docs.txt: https://github.com/ImagingDataCommons/highdicom/blob/3f7a507958a49f7a2438f492a6a3a7a70eb3a45d/.readthedocs.yaml#L27

CPBridge commented 3 months ago

Yeah I just noticed that too, I'll take care of it. Thanks!

DimitriPapadopoulos commented 3 months ago

Perhaps you need to restore requirements_docs.txt. It looks like pyproject.toml won't be supported by readthedocs: https://github.com/readthedocs/readthedocs.org/issues/8599

DimitriPapadopoulos commented 3 months ago

My wrong, this is covered by packages:

version: 2

python:
  install:
    - method: pip
      path: .
      extra_requirements:
        - docs

With the previous settings, Read the Docs will execute the next commands:

pip install .[docs]
CPBridge commented 3 months ago

Yup, I came across and implemented the same solution. Builds just passed on readthedocs