ENH: Move BlackIs1 functionality to tiff_header

[j-t-1]

Also correct types in CCITTParameters.

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 96.97%. Comparing base (0b58493) to head (4555d87).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #3421   +/-   ##
=======================================
  Coverage   96.97%   96.97%           
=======================================
  Files          54       54           
  Lines        9323     9324    +1     
  Branches     1707     1708    +1     
=======================================
+ Hits         9041     9042    +1     
  Misses        168      168           
  Partials      114      114           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[j-t-1]

It would be easier to have defaults in CCITTParameters for columns. Then we do not have to set them in _get_parameters.

pypdf/pypdf/filters.py

Lines 589 to 596 in 56f0eaa

	@staticmethod
	def _get_parameters(
	parameters: Union[None, ArrayObject, DictionaryObject, IndirectObject],
	rows: Union[int, IndirectObject],
	) -> CCITTParameters:
	# §7.4.6, optional parameters for the CCITTFaxDecode filter
	k = 0
	columns = 1728

But then if they are not retrieved they would be None. The design of this could be better.

[stefan6419846]

But then if they are not retrieved they would be None. The design of this could be better.

This is wrong. The default value currently seems to be 0 instead of 1728.

The main change we would have to do here would be creating the dataclass instance at the beginning and writing its attributes directly.

[j-t-1]

But then if they are not retrieved they would be None. The design of this could be better.

This is wrong. The default value currently seems to be 0 instead of 1728.

The main change we would have to do here would be creating the dataclass instance at the beginning and writing its attributes directly.

So we need to change the default to be 1728. Then initiate the dataclass, obtaining the defaults from CCITTParameters? If all the defaults (from the specification) are there then it is easier to understand.

[j-t-1]

FAILED tests/test_filters.py::test_ccitt_get_parameters[None-0] - UnboundLocalError: cannot access local variable 'black_is_1' where it is not associated with a value

The test does not use the new variable. Is this error saying although it is unused in the test it needs to be used, kinda like coverage tests?

[stefan6419846]

The test does not use the new variable. Is this error saying although it is unused in the test it needs to be used, kinda like coverage tests?

You never initialize this variable in the _get_parameters method. If the BlackIs1 parameter is omitted in the PDF, this will fail.

This should be solved by implementing the proposal from #3421 (comment) as well.

[j-t-1]

@stefan6419846 do you know how to fix the new set of errors?

[stefan6419846]

Not directly. You might want to try setting int(params.BlackIs1) to fixed 0 again. If this works, this most likely indicates that Pillow somehow refuses to load such TIFF images directly and we would most likely have to revert/ignore most of the changes of this PR - or maybe we find a way to further discuss this with upstream if I find the time to have a deeper look into this myself.

[j-t-1]

Agree to fix int(params.BlackIs1) to 0 (it worked before). The changes we have made in this PR make it more understandable than before. We may be able to get this over the line.

rows in _get_parameters is unused, have put that back.