Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failure with Python 3.12 but not Python 3.11: no attribute 'delete' #1642

Closed
mcarans opened this issue Feb 15, 2024 · 9 comments · Fixed by #1684
Closed

Failure with Python 3.12 but not Python 3.11: no attribute 'delete' #1642

mcarans opened this issue Feb 15, 2024 · 9 comments · Fixed by #1684
Labels
bug Something isn't working

Comments

@mcarans
Copy link

mcarans commented Feb 15, 2024

I have found an issue with frictionless with Python 3.12 that doesn't occur with Python 3.11. Here is the trace:

../../../.local/share/hatch/env/virtual/hdx-python-utilities/uS5tbIcO/test.py3.12/lib/python3.12/site-packages/frictionless/formats/excel/parsers/xlsx.py:67: in read_loader
    if not target.delete:
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <tempfile._TemporaryFileWrapper object at 0x7ff2d54ab350>
name = 'delete'

    def __getattr__(self, name):
        # Attribute lookups are delegated to the underlying file
        # and cached for non-numeric results
        # (i.e. methods are cached, closed and friends are not)
        file = self.__dict__['file']
>       a = getattr(file, name)
E       AttributeError: '_io.BufferedRandom' object has no attribute 'delete'

It can be reproduced with:

from frictionless import Detector, Dialect
from frictionless.formats import ExcelControl
from frictionless.resources import TableResource

url = "https://raw.githubusercontent.com/OCHA-DAP/hdx-python-utilities/main/tests/fixtures/downloader/test_xlsx_processing.xlsx?a=1"

control = ExcelControl()
control.sheet = "Trend"
control.fill_merged_cells = True

detector = Detector()
detector.field_type = "any"
detector.field_float_numbers = True
detector.field_missing_values = [""]

dialect = Dialect()
dialect.header = True
dialect.header_rows = [7, 8]
dialect.skip_blank_rows = True

resource = TableResource(
    path=url,
    format="xlsx",
    control=control,
    detector=detector,
    dialect=dialect,
)
resource.open()
@mcarans
Copy link
Author

mcarans commented Feb 20, 2024

@roll Just bringing this issue to your attention as I think it will affect anyone reading Excel with frictionless in Python 3.12.

@mcarans
Copy link
Author

mcarans commented Apr 29, 2024

@roll I tested with frictionless 5.17.0 and unfortunately it doesn't fix this issue

@roll
Copy link
Member

roll commented Apr 30, 2024

Thanks @mcarans!

Yea, I know, I think they found the reason in this thread - Tinche/aiofiles#166

cc @pdelboca

@mcarans
Copy link
Author

mcarans commented Sep 5, 2024

@roll @pdelboca
Tested with 5.17.1 and it still fails.

@pierrecamilleri
Copy link
Collaborator

pierrecamilleri commented Sep 9, 2024

Hi @mcarans, can't reproduce (tried to validate an .xlsx file with python 3.12.6 + frictionless 5.17.1).
Could you please provide data and the command to reproduce ? Thanks.

@mcarans
Copy link
Author

mcarans commented Sep 9, 2024

@pierrecamilleri This will reproduce it:

from frictionless import Detector, Dialect
from frictionless.formats import ExcelControl
from frictionless.resources import TableResource

url = "https://raw.githubusercontent.com/OCHA-DAP/hdx-python-utilities/main/tests/fixtures/downloader/test_xlsx_processing.xlsx?a=1"

control = ExcelControl()
control.sheet = "Trend"
control.fill_merged_cells = True

detector = Detector()
detector.field_type = "any"
detector.field_float_numbers = True
detector.field_missing_values = [""]

dialect = Dialect()
dialect.header = True
dialect.header_rows = [7, 8]
dialect.skip_blank_rows = True

resource = TableResource(
    path=url,
    format="xlsx",
    control=control,
    detector=detector,
    dialect=dialect,
)
resource.open()

@pierrecamilleri
Copy link
Collaborator

Thanks 🙏 , I can reproduce

@pierrecamilleri
Copy link
Collaborator

I'm sorry, I just realized that the reproducible example was already in the ticket... I must have been tired...

I looked at the faulty line, and it looks like it is using some delete property that moved in python 3.12 (in this commit) to _closer.delete.

The fix should be simple (we already have the value of "delete" and there is no reason to access it again through a private property), I will make a PR after double checking.

@pierrecamilleri
Copy link
Collaborator

Sorry, it took some time as I wanted to understand why pyright's type checking (run in CI with python 3.12) would not intercept this error.

It looks like a limitation of stdlib typestubs : see this issue.

pierrecamilleri added a commit that referenced this issue Sep 27, 2024
- fixes #1642 

Adapt to changes of API of `_TemporaryFileWrapper` in python 3.12,
unfortunately not intercepted by pyright (see issue for details). The
fix does not rely on this class's API, as the value is already stored in
a local variable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants