Default `allow_parallel_execution=True` about python-bibtexparser HOT 3 CLOSED

Hi.

First: My answer here also considers PR #459 and #460.

The choice to have users define this property explicitly, especially when overriding middleware, was made on purpose. I wanted to put an emphasis on the fact that there should be an option to not change the library passed as input. However, provided with the reactions received so far, and a fresh look over a year from implementing this I am happy to back off while I still can :-)

I am still not merging your PRs just yet for the following reasons:

1. What's the reason for opening two distinct PRs? They appear to belong together. I do not particularly mind merging two PRs, but I am just wondering if I am missing a point. Is it because most Middleware already has the default set, but not the ones in #459?
2. In #460: Any reason why you made this a default in BlockMiddleware, but not in the superclass Middleware or the sister class LibraryMiddleware?
3. The docs (https://github.com/sciunto-org/python-bibtexparser/blob/main/docs/source/customize.rst) should probably be slightly extended and adapted to reflect these changes.

from python-bibtexparser.

1. What's the reason for opening two distinct PRs? They appear to belong together. I do not particularly mind merging two PRs, but I am just wondering if I am missing a point. Is it because most Middleware already has the default set, but not the ones in Adding missing default allow_inplace_modification. Simplifying examples #459?

The two concern different options. #459 just makes existing code consistent. Instead, #460 applies a default that was not there yet.

2. In BlockMiddleware: default allow_parallel_execution=True #460: Any reason why you made this a default in BlockMiddleware, but not in the superclass Middleware or the sister class LibraryMiddleware?

Will fix.

3. The docs (https://github.com/sciunto-org/python-bibtexparser/blob/main/docs/source/customize.rst) should probably be slightly extended and adapted to reflect these changes.

Agreed. I get the setting the default allow_parallel_execution=True requires a bit more caution on the user side, so clear documentation is indeed important. Do you happen to have a simple example in mind for when allow_parallel_execution=True would break things?

from python-bibtexparser.

Keeping the discussion centralized:

Point 2 is (and was) resolved:

• LibraryMiddleware does not have the allow_parallel_execution option:

  python-bibtexparser/bibtexparser/middlewares/middleware.py

  Lines 206 to 209 in e3757c1

  	super().__init__(
  	allow_inplace_modification=allow_inplace_modification,
  	allow_parallel_execution=False,
  	)

• #460 sets the default on Middleware.

• BlockMiddleware has its own constructor.

Point 3: I added a documentation commit to #460

from python-bibtexparser.