Skip to content

FIX: account for removals in py313 in importlib#11998

Closed
tacaswell wants to merge 3 commits intomesonbuild:masterfrom
tacaswell:mnt/py313_compat
Closed

FIX: account for removals in py313 in importlib#11998
tacaswell wants to merge 3 commits intomesonbuild:masterfrom
tacaswell:mnt/py313_compat

Conversation

@tacaswell
Copy link

243fdcb40ebeb177ce723911c1f7fad8a1fdf6cb /
python/cpython#106532 removes functions deprecated in py311.

@tacaswell tacaswell requested a review from jpakkane as a code owner July 16, 2023 22:27
@eli-schwartz
Copy link
Member

Oh dear, this is disappointing.

Why is this removed. Why does importlib.resources go around deleting APIs practically the instant that a replacement appears? Will we ever achieve stability? It is now impossible to write portable code that targets all supported versions of python without ifdef hackery.

I hate the necessity of this and would dearly like for it to be resolved by CPython reverting this.

@tacaswell
Copy link
Author

I have no answers to these questions, I just have a set of scripts (https://github.com/tacaswell/build_the_world) that tries to build the software stack I care about from the default branches of everything with the intention of catching things like this as early as possible.

I have no strong views on if it is better for meson to take this patch or CPython to defer the removal longer (so long as numpy will build 😄).

@eli-schwartz
Copy link
Member

Sure, I get that. :)

Please note there's another hidden use of this in mesonbuild/mesondata.py as it turns out, so that should be fixed at the same time. In that case, it's importlib.resources.read_text but that function is in the same boat.

@tacaswell
Copy link
Author

Possibly surprising that does not get hit in any of the Python projects that have moved over already.

@eli-schwartz
Copy link
Member

That could be because it's only used in the cmake handling code.

On that note, I found another couple instances:

  • mesonbuild/dependencies/cmake.py: cmake_txt = importlib.resources.read_text('mesonbuild.dependencies.data', cmake_file, encoding = 'utf-8')
  • mesonbuild/modules/python.py: f.write(importlib.resources.read_binary('mesonbuild.scripts', 'pycompile.py'))

@xclaesse
Copy link
Member

xclaesse commented Jul 18, 2023

Wondering if we should add compat layer in e.g. utils/resources.py to avoid adding if-else everywhere.

import mesonlib
data = mesonlib.resources.read_text(package, resource)

@eli-schwartz
Copy link
Member

I don't think that really helps considering that we need 3 different APIs across 4 locations. There's extremely little duplication here.

Simply hiding if/else in another file doesn't help much IMO.

@tacaswell
Copy link
Author

@eli-schwartz I now think I have gotten them all.

@tacaswell
Copy link
Author

@eli-schwartz Is there anything else to do here?

@eli-schwartz
Copy link
Member

I opened a proposal for importlib-resources, with the goal of streamlining the API -- maybe even for 3.13!

I really need to get back to that. I'll put it in my priority list.

@eli-schwartz
Copy link
Member

The functional API is on track to be added back into 3.13 alpha and un-deprecated, which would mean that no released version of python will have lacked it.

Once it is merged into CPython I propose to close this PR.

@eli-schwartz
Copy link
Member

The CPython PR was merged and the API is restored to the 3.13 alpha.

@tacaswell tacaswell deleted the mnt/py313_compat branch May 22, 2024 01:41
@tacaswell
Copy link
Author

I can confirm that the default branch works again with the 3.13 CPYthon branch.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants