Default toc length config #2042

voisardf · 2024-08-27T15:00:45Z

A new parameter default_toc_length in the print configuration allows to deactivate the compute_toc_pages function and set a default toc page length which applies in almost every case (exceptions, see description)

new print config parameter(s)
tests

The TOC page breaks depend on the overall number of published topics, the settings regarding the land register elements and the individual disclaimer options (QR code).
In Neuchâtel, with only one or two concerned themes, we get one TOC page, with three or more concerned topics the TOC will be at least two pages.

codecov · 2024-08-28T15:14:29Z

Codecov Report

Attention: Patch coverage is 59.25926% with 11 lines in your changes missing coverage. Please review.

Project coverage is 85.45%. Comparing base (86fe195) to head (ea54626).
Report is 19 commits behind head on master.

Files with missing lines	Patch %	Lines
...contrib/print_proxy/mapfish_print/mapfish_print.py	59.25%	11 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##           master    #2042      +/-   ##
==========================================
- Coverage   85.51%   85.45%   -0.07%     
==========================================
  Files         120      120              
  Lines        5276     5281       +5     
==========================================
+ Hits         4512     4513       +1     
- Misses        764      768       +4

Flag	Coverage Δ
unittests	`85.45% <59.25%> (-0.07%)`	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

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

svamaa · 2024-08-29T12:34:14Z

pyramid_oereb/contrib/print_proxy/mapfish_print/mapfish_print.py

@@ -116,6 +119,7 @@ def __call__(self, value, system):
            data=json.dumps(spec)
        )
        try:
+            log.debug('Validation of the TOC length with compute_toc_pages set to {} and default_toc_length set to {}'.format(print_config.get('compute_toc_pages'), print_config.get('default_toc_length'))) # noqa
            if Config.get('print', {}).get('compute_toc_pages', False):


we can leave the if statement away.
The check if the toc pages from the actual pdf fits the extract_as_dict['nbTocPages'] we've set above should be done anyways and will be the same for all three cases.

svamaa · 2024-08-29T12:36:56Z

I've tested this PR with compute_toc_pages=false and default_toc_length=2 and found a 30% faster pdf extract!
Great work! thank you @voisardf

svamaa · 2024-08-29T12:45:05Z

dev/config/pyramid_oereb.yml.mako

@@ -88,6 +88,16 @@ pyramid_oereb:
    # more time to generate the PDF. If set to false, it will assume that only one TOC page exists, and this can
    # lead to wrong numbering in the TOC.
    compute_toc_pages: true
+    # To avoid the potentially time consuming second computing of the PDF extract and skip the the computation


maybe the in the comment, you could point out, that if the compute_toc_pages is set to true, it will always overwrite the default_toc_length.

michmuel · 2024-08-29T12:48:14Z

dev/config/pyramid_oereb.yml.mako

@@ -88,6 +88,16 @@ pyramid_oereb:
    # more time to generate the PDF. If set to false, it will assume that only one TOC page exists, and this can
    # lead to wrong numbering in the TOC.
    compute_toc_pages: true
+    # To avoid the potentially time consuming second computing of the PDF extract and skip the the computation


Maybe:

In order to skip the computation of the estimated number of TOC pages which might return an erroneous result for your setting, you can specify a default for the number of TOC pages. For most of the cantons the number of TOC pages is pretty constant unless a real estate is concerned by none or a huge number of restrictions.
In both cases (computing an estimate or setting a default for the number of TOC pages) the exact number of TOC pages is extracted from the created PDF and if it differs from the expected value the PDF is created a second time with the correct page numbers.
Note that if "compute_toc_pages" is set true the "default_toc_length" is not taken into account.

michmuel · 2024-08-29T13:06:17Z

doc/source/changes.rst

@@ -6,6 +6,13 @@ Changes/Hints for migration
 This chapter will give you hints on how to handle version migration, in particular regarding what you may need
 to adapt in your project configuration, database etc. when upgrading to a new version.

+Version 2.6.0
+-------------
+* New parameter 'default_toc_length' allows to define a default table of content pages number avoiding a second


I would omit the statemtents about the 95% and the default setting. I guess default is to not set this parameter.

michmuel · 2024-08-29T13:07:07Z

dev/config/pyramid_oereb.yml.mako

+    # In both cases (computing an estimated length or setting a default length) the exact number of TOC pages is
+    # extracted from the created PDF and if it is different from the expected value the PDF extract is called a 
+    # second time with the correct page numbers.
+    default_toc_length: 2


Maybe the name of the variable could be more precise. E.g. "most_frequent_toc_length"

michmuel · 2024-08-29T13:12:05Z

pyramid_oereb/contrib/print_proxy/mapfish_print/mapfish_print.py

@@ -73,17 +73,20 @@ def __call__(self, value, system):
        extract_as_dict = self._render(extract_record, value[1])
        feature_geometry = mapping(extract_record.real_estate.limit)

+        print_config = Config.get('print', {})


If the variable 'print_config' is defined, it should thoroughly be used in subsequent parts of the code.

michmuel · 2024-08-29T13:13:47Z

pyramid_oereb/contrib/print_proxy/mapfish_print/mapfish_print.py

        if Config.get('print', {}).get('compute_toc_pages', False):
            extract_as_dict['nbTocPages'] = TocPages(extract_as_dict).getNbPages()
        else:
-            extract_as_dict['nbTocPages'] = 1
+            if Config.get('print', {}).get('default_toc_length', False):
+                extract_as_dict['nbTocPages'] = print_config.get('default_toc_length', 2)


I would not specify a default, but test if it is a positive number.

michmuel · 2024-08-29T13:20:07Z

pyramid_oereb/contrib/print_proxy/mapfish_print/mapfish_print.py

@@ -131,14 +135,43 @@ def __call__(self, value, system):
                    except ValueError:
                        true_nb_of_toc = 1

+                    log.debug('True number of TOC pages is {}'.format(true_nb_of_toc))


Maybe the expected number can also be written to the log (in case it is computed).

michmuel · 2024-08-29T13:22:06Z

pyramid_oereb/contrib/print_proxy/mapfish_print/mapfish_print.py

@@ -116,6 +119,7 @@ def __call__(self, value, system):
            data=json.dumps(spec)
        )
        try:
+            log.debug('Validation of the TOC length with compute_toc_pages set to {} and default_toc_length set to {}'.format(print_config.get('compute_toc_pages'), print_config.get('default_toc_length'))) # noqa
            if Config.get('print', {}).get('compute_toc_pages', False):


michmuel · 2024-08-29T13:25:14Z

Thanks for the pull request! Looks good to me. I added some remarks, some of them are discussible.

voisardf · 2024-08-29T15:07:58Z

@svamaa @michmuel I adapted the code with most of your suggestions. Thanks!

lopo977

Looks good to me.

svamaa · 2024-08-30T09:25:27Z

Looks good to me! Thank you @voisardf !

michmuel

Looks good to me. Thanks! I noted one small thing (maybe its good to change it here in order to not forget it).

michmuel · 2024-08-30T09:47:23Z

doc/source/changes.rst

+* New parameter 'expected_toc_length' allows to define a default table of content pages number avoiding a second
+call for the pdf extract in most cases. This value should be set if most of the PDF extracts have the same number
+of TOC pages.
+Default setting: 2


That is omitted now.

A change in this pull request is that the pdf is built a second time whenever the numbers of toc pages disagree between extract_as_dict['nbTocPages'] (computed/expected/1, depending on the setting) and the one in the returned pdf.

In the current version it is only rebuild if 'compute_toc_pages' is true.

voisardf · 2024-09-04T07:11:03Z

thanks for all your input!

voisardf added 8 commits November 28, 2023 10:18

Merge branch 'master' of https://github.com/openoereb/pyramid_oereb

0b56eea

Merge branch 'master' of https://github.com/openoereb/pyramid_oereb

be33be2

Merge remote-tracking branch 'upstream/master'

b84a487

Merge remote-tracking branch 'upstream/master'

df0c13f

Merge branch 'master' of https://github.com/openoereb/pyramid_oereb

d667506

Merge remote-tracking branch 'upstream/master'

46444b5

Merge branch 'master' of https://github.com/openoereb/pyramid_oereb

f50347b

first draft of new toc parameters implementation

69e23c4

voisardf added enhancement mapfish print usergroup work This lable is used to mark issues which will be done by usergroup members labels Aug 27, 2024

voisardf self-assigned this Aug 27, 2024

Refactured the process and added more logging

3b55d99

voisardf added 4 commits August 28, 2024 17:26

added linting comments

d8c72db

added two config tests

7296571

fixed some test and linting

087b45d

more linting, sorry

c4b9261

voisardf marked this pull request as ready for review August 29, 2024 10:20

improved and fixed parameter documentation

e9023cb

voisardf force-pushed the default_toc_length_config branch from 5ac0b16 to e9023cb Compare August 29, 2024 11:35

voisardf requested review from peterschaer, lopo977, michmuel and svamaa August 29, 2024 11:38

svamaa requested changes Aug 29, 2024

View reviewed changes

svamaa reviewed Aug 29, 2024

View reviewed changes

michmuel requested changes Aug 29, 2024

View reviewed changes

improved code as discussed and suggested

a5157d0

typo with lint

77f6db1

lopo977 approved these changes Aug 30, 2024

View reviewed changes

renamed parameter as requested

a5f42fa

michmuel requested changes Aug 30, 2024

View reviewed changes

adapted documentation of new parameter

ea54626

voisardf merged commit a6c972f into master Sep 4, 2024
10 of 12 checks passed

voisardf deleted the default_toc_length_config branch September 4, 2024 07:11

voisardf mentioned this pull request Sep 4, 2024

TOC - fixed TOC pages in configuraiton #2039

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Default toc length config #2042

Default toc length config #2042

voisardf commented Aug 27, 2024 •

edited

Loading

codecov bot commented Aug 28, 2024 •

edited

Loading

svamaa Aug 29, 2024

michmuel Aug 29, 2024

svamaa commented Aug 29, 2024

svamaa Aug 29, 2024

michmuel Aug 29, 2024

michmuel Aug 29, 2024

michmuel Aug 29, 2024

michmuel Aug 29, 2024

michmuel Aug 29, 2024

michmuel Aug 29, 2024

michmuel Aug 29, 2024

michmuel commented Aug 29, 2024

voisardf commented Aug 29, 2024

lopo977 left a comment

svamaa commented Aug 30, 2024

michmuel left a comment

michmuel Aug 30, 2024

michmuel Aug 30, 2024

voisardf commented Sep 4, 2024

Default toc length config #2042

Default toc length config #2042

Conversation

voisardf commented Aug 27, 2024 • edited Loading

codecov bot commented Aug 28, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

svamaa commented Aug 29, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

michmuel commented Aug 29, 2024

voisardf commented Aug 29, 2024

lopo977 left a comment

Choose a reason for hiding this comment

svamaa commented Aug 30, 2024

michmuel left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

voisardf commented Sep 4, 2024

voisardf commented Aug 27, 2024 •

edited

Loading

codecov bot commented Aug 28, 2024 •

edited

Loading