Tests to follow up on

[darcymason]

In working on the github actions workflows, I've been looking at what tests cover what, to try to minimize the CI time.

I've collected some tests that could use further investigation (may be okay, but putting it in an issue to possibly come back to later):

SKIPPED in all tests:

• tests\test_values.py:44: bad bytestring not handled properly
• tests\test_values.py:179: bad bytestring not handled properly
• tests\test_values.py:38: empty bytestring not handled properly
• tests\test_filereader.py:1284: No public elements with VR of SV

These are all pytest.mark.skip, i.e. no conditions, so at some point were just a 'not implemented', I guess.

XFAILED

(marked as expected to fail):
These are all under tests/test_encoders.py::TestEncoder_Encode::

• test_enc_dataset_multiframe - Changing compression not implemented yet
• test_enc_dataset_multiframe_no_idx_raises - Changing compression not implemented yet
• test_enc_dataset - Changing compression not implemented yet
• test_enc_dataset_specific_enc - Changing compression not implemented yet
• test_enc_dataset_specific_dec - Changing compression not implemented yet
• test_enc_iter_encode - Changing compression not implemented yet

XPASSED:

(expected to fail but did not; I've collapsed some of the test path/name and file paths for readability)
These are all under TestPillowHandler_JPEG::test_array[pydicom\data\test_files\

• SC_rgb_dcmtk_+eb+cy+n1.dcm-...data_store\data\SC_rgb_dcmtk_ebcyn1_dcmd.dcm-values2] Resulting image is a bad match
• SC_rgb_dcmtk_+eb+cy+n1.dcm-.pydicom\data\SC_rgb_dcmtk_ebcyn1_dcmd.dcm-values2] Resulting image is a bad match
• SC_rgb_dcmtk_+eb+cy+np.dcm-...data_store\data\SC_rgb_dcmtk_ebcynp_dcmd.dcm-values0] Resulting image is a bad match
• SC_rgb_dcmtk_+eb+cy+np.dcm-.pydicom\data\SC_rgb_dcmtk_ebcynp_dcmd.dcm-values0] Resulting image is a bad match

The xfailed look fine, actually, we could just leave them (or delete for now and pull them back from old commits if ever wanted them).

The xpassed are probably fine too, just a few out of a matrix that worked?

[mrbean-bremen]

About the skipped tests: the first three indeed look like we didn't implemwnt the use case - if it is a valid use case (I'm not sure yet), we should just implement it.
I don't understand why the SV test is skipped - it passes for me, and I don't understand the skip message.

I agree that the xfailed tests may remain - I'm not sure when we will implement compression (or who will do it), but this is certainly something we want to have in pydicom.

As for the xpassed Pillow tests: I already mentioned elsewhere that I would like to remove Pillow altogether from the supported image plugins. Pillow has several problems, and we now have the pylibjpeg plugins, also pygdcm is much easier to install. I think this would be a reasonable breaking change for 3.0. Ehat do you think?

[darcymason]

As for the xpassed Pillow tests: I already mentioned elsewhere that I would like to remove Pillow altogether from the supported image plugins. Pillow has several problems, and we now have the pylibjpeg plugins, also pygdcm is much easier to install. I think this would be a reasonable breaking change for 3.0. What do you think?

Yes, I have been keeping in mind your suggestion before to remove Pillow. I agree, if it is going to happen, it should be now. I've got another PR coming with the workflow updates, I'll leave it in for that one, just to compare with previous, then we can do another PR with just the Pillow removal (and all its tests and docs).

[scaramallion]

The SV test was skipped because at the time there were no public elements that used that VR