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

Rearrange a bit and make it abi3 compatible. #6

Merged
merged 34 commits into from
Apr 25, 2024

Conversation

Sachaa-Thanasius
Copy link
Contributor

@Sachaa-Thanasius Sachaa-Thanasius commented Apr 22, 2024

This way, the package doesn't need to be recompiled for any future minor releases.

However, this particular pull request changes a lot related to package config/setup, which I know shouldn't be done since it's personal preference. Didn't go through the effort of separating that back out yet, but it can be fixed if it means the PR will be considered.

Sachaa-Thanasius and others added 8 commits April 19, 2024 11:35
- Put source files in one package.
- Add an __init__.py and __init__.pyi to provide a target for type stubs.
 - Probably too eager; typeshed will probably provide these until 3.12 is EOL.
- Change setup configuration to
 a. Put as much as possible in pyproject.toml.
 b. Eliminate the need for setup.cfg.
 c. Don't break everything when attempting to build with `build` — the audioop header file just wasn't being seen.
- Most importantly, make it abi3. i.e., the resulting wheel should be compatible for >=3.12 environments.
Used the wrong one out of habit.
- More of a footgun than anything.
@Sachaa-Thanasius
Copy link
Contributor Author

Sachaa-Thanasius commented Apr 22, 2024

Assuming this functionality were added, it might be prudent for the README to have a warning and an example for making sure this is only installed on python 3.13+.

For example, borrowing language from a similar third-party backport for a PEP 594 dead battery:

Because audioop is still a valid library in all relevant versions of the standard library up to and including Python 3.12, care should be taken to avoid a conflict when installing and importing it. For example, use this line in your install_requires, project.dependencies section, requirements.txt file, etc.:

'audioop; python_version>="3.13"'

Maybe after 3.13 is released officially, the project metadata would then be updated to put the lower bound at 3.13, avoiding the possible footgun.

Not sure if including that is within the scope of this PR, but it's still maybe something worthy of consideration.

@AbstractUmbra
Copy link
Owner

Assuming this functionality were added, it might be prudent for the README to have a warning and an example for making sure this is only installed on python 3.13+.

For example, borrowing language from a similar third-party backport for a PEP 594 dead battery:

Because audioop is still a valid library in all relevant versions of the standard library up to and including Python 3.12, care should be taken to avoid a conflict when installing and importing it. For example, use this line in your install_requires, project.dependencies section, requirements.txt file, etc.:

'audioop; python_version>="3.13"'

Maybe after 3.13 is released officially, the project metadata would then be updated to put the lower bound at 3.13, avoiding the possible footgun.

Not sure if including that is within the scope of this PR, but it's still maybe something worthy of consideration.

This was an oversight by me, feel free to edit the relevant library meta to only allow 3.13+ as well as the README with the changes presented here.

@Sachaa-Thanasius
Copy link
Contributor Author

Oh, coolio. I'll get on that.

@AbstractUmbra
Copy link
Owner

Oh, coolio. I'll get on that.

Further note, sorry, if the library meta only allows installation on 3.13+ the warning in the readme is not relevant, aha.

- Add a note in setup.py about the limited API compatibility being more permissive but would lead to potential clobbering if requires-python matched it.
@Sachaa-Thanasius
Copy link
Contributor Author

Sachaa-Thanasius commented Apr 22, 2024

Aight, fixed. On a related note, the idea came to mind to add a link to PEP 594 and/or the stdlib audioop docs (or even copying the docs .rst file in), but that isn't really in scope for this PR either. Figured I'd still bring it up.

EDIT: Also maybe preserving the git history somehow (see the tail end of this post and a potential recipe a bit further down in the thread).

@AbstractUmbra
Copy link
Owner

I've caused a merge conflict by updating main with the current metadata needs, you'll need to rebase and resolve the merge I think, sorry about that!

@Sachaa-Thanasius
Copy link
Contributor Author

Sachaa-Thanasius commented Apr 23, 2024

Let's see if I broke it.

EDIT: That's a yes. Not sure how to apply the removal of the setup.cfg on top of everything. Any tips?

- Put source files in one package.
- Add an __init__.py and __init__.pyi to provide a target for type stubs.
 - Probably too eager; typeshed will probably provide these until 3.12 is EOL.
- Change setup configuration to
 a. Put as much as possible in pyproject.toml.
 b. Eliminate the need for setup.cfg.
 c. Don't break everything when attempting to build with `build` — the audioop header file just wasn't being seen.
- Most importantly, make it abi3. i.e., the resulting wheel should be compatible for >=3.12 environments.
Sachaa-Thanasius and others added 7 commits April 23, 2024 10:41
Used the wrong one out of habit.
- More of a footgun than anything.
- Add a note in setup.py about the limited API compatibility being more permissive but would lead to potential clobbering if requires-python matched it.
@Sachaa-Thanasius
Copy link
Contributor Author

Sachaa-Thanasius commented Apr 23, 2024

Yeah, this commit log is ugly as sin, even if it worked. Sorry about that; my git knowledge isn't the best. @AbstractUmbra

I guess squashing it is the only way to clean it up?

… tag) to be 3.13.

- This should be the final necessary measure to avoid clobbering.
- Typeshed has argument types as `bytes` to substitute for buffers since buffers didn't have a representation in the type system for a long time. Now that `collections.abc.Buffer` exists, there's a better alternative.
 - This means that memoryview, bytarray, etc. won't show up as type-checker errors when given as inputs.
- Tests actually type-check as a result. No changes to logic were made, just type annotations.
- TODO: Consider upstreaming to typeshed?

Reference material:
- https://docs.python.org/3/library/typing.html#typing.ByteString
- https://docs.python.org/3/library/collections.abc.html#collections.abc.Buffer
- https://peps.python.org/pep-0688
 - "No special meaning for bytes" mentions why these were annotated with `bytes` before.
- https://typing.readthedocs.io/en/latest/source/modernizing.html#typing-bytestring
@Sachaa-Thanasius
Copy link
Contributor Author

Anything else that needs to be considered or is on a wishlist for this?

@AbstractUmbra
Copy link
Owner

Anything else that needs to be considered or is on a wishlist for this?

Looks all good, thanks!

@AbstractUmbra AbstractUmbra merged commit af2305b into AbstractUmbra:main Apr 25, 2024
4 checks passed
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.

None yet

2 participants