MeirKriheli/python-bidi

Non backward compatible with 0.4.2

Closed this issue · 11 comments

from bidi import get_display in 0.5.0
from bidi.algorithm import get_display in 0.4.2

File "/usr/local/lib/python3.8/site-packages/xhtml2pdf/util.py", line 25, in
from bidi.algorithm import get_display
ModuleNotFoundError: No module named 'bidi.algorithm'

xhtml2pdf latest (0.2.16) version also has

# Copyright 2010 Dirk Holtwick, holtwick.it
#
# Licensed under the Apache License, Version 2.0 (the "License");
# you may not use this file except in compliance with the License.
# You may obtain a copy of the License at
#
#     http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
from __future__ import annotations

import contextlib
import logging
import re
from copy import copy
from typing import Any

import arabic_reshaper
import reportlab
import reportlab.pdfbase._cidfontdata
from bidi.algorithm import get_display

change your last line code to: from bidi import get_display. for me it fixed the issue.

It's was in the release notes.

Released 0.5.1 preserving older import path.

It's was in the release notes.

Released 0.5.1 preserving older import path.

Yes it was. But unless you are picking this up directly, html2pdf is picking it up and they seem to be using >=0.4.2. So they picked the latest and since this is picked 3-4 levels down the transitive chain, the higher level teams wouldn't be aware till it breaks.

Thanks for releasing 0.5.1

Yeah, in that case it's a bad version pinning on their behalf, should've been >=0.4.2,<0.5.0

Yeah, in that case it's a bad version pinning on their behalf, should've been >=0.4.2,<0.5.0

I suppose so. However, since you are using magic version, I think == 0.4.2 is the correct way?

Also lost: get_base_level.

And in contrast to get_display, that cannot be imported from elsewhere – it's completely gone. It's also not in f9dd9cc.

Please re-open.

See here for our usage context.

get_base_level is an implementation detail. It was not documented as an external api, did not have tests etc.

I wanted to implemented a different, public function (which returns meaningful values, not int).

I'll try to add one which will work like the older one, but that's the extent of it. I'm not gonna implement any other of the internal functions used to implement the prev algorithm.

get_base_level is an implementation detail. It was not documented as an external api, did not have tests etc.

I wanted to implemented a different, public function (which returns meaningful values, not int).

Oh, sorry, I did not know that.

I'll try to add one which will work like the older one, but that's the extent of it. I'm not gonna implement any other of the internal functions used to implement the prev algorithm.

Ok, thanks!

So how would you recommend to rewrite code that heavily uses these other functions like this?

Wow, that goes pretty deep into the algo, I don't know what can be done.

One option I guess could be vendoring in the old code (licensing may conflict though).

Can you explain what exactly it tries to achieve? Maybe something can be worked out.

@MeirKriheli closed this as completed in 1ab5cad 45 minutes ago

thanks!

Wow, that goes pretty deep into the algo, I don't know what can be done.

One option I guess could be vendoring in the old code (licensing may conflict though).

Can you explain what exactly it tries to achieve? Maybe something can be worked out.

Only @mittagessen can explain. (Kraken is a BiDi-aware multilingual OCR engine.)

The author of the code here.

Wow, that goes pretty deep into the algo, I don't know what can be done.

Probably not much. I'm abusing the storage dict quite badly to have my metadata reordered as well. There are some functions in the rust bidi_unicode module that would make my code much cleaner but it would be annoying to expose them.

Digging through the docs of bidi_unicode I've also realized that the 0.5 releases of python-bidi introduce a fairly major regression. The rust module doesn't run rules L3 and L4 while the old python implementation implements them. I've opened a separate bug report for that.

One option I guess could be vendoring in the old code (licensing may conflict though).

Yes, the LGPL is somewhat bothersome for integration into an Apache project. I'll just keep it pinned for now until I can find time for a reimplementation from scratch as the rust module lacks half of the Bidi algorithm anyway.