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

pyktx (Python bindings for libktx) #698

Merged
merged 138 commits into from
Nov 29, 2023
Merged

pyktx (Python bindings for libktx) #698

merged 138 commits into from
Nov 29, 2023

Conversation

ShukantPal
Copy link
Contributor

@ShukantPal ShukantPal commented May 8, 2023

Sorry re-opening #665 after I force pushed & accidentally deleted that PR. The history hasn't been changed

- Create Python project
- Setup CFFI for C bindings
- Write glue code in C for some property getters and calling native functions
- KtxTexture, KtxTexture1, KtxTexture2 Python classes calling into glue code and directly into library
- Other data classes & enums from libktx in Python code
Pass in C_INCLUDE_PATH, LDFLAGS when building
@ShukantPal
Copy link
Contributor Author

Hi Mark, sorry for the delay. I was flying back home yesterday for the holidays. I also turned 21 today!

Here's what you have to put in the workflow:

on:
  workflow_dispatch: # this should exist already
    inputs: # define inputs
      test_pypi: # usage in your workflow ${{ inputs.test_pypi }}
        type: boolean
        description: 'Deploy to test pypi registry'
        required: true
        default: false

For more details: https://github.blog/changelog/2021-11-10-github-actions-input-types-for-manual-workflows/

@MarkCallow
Copy link
Collaborator

Hi @ShukantPal, I Hope you have a great holiday season and I belated wish you Happy Birthday. Thanks for the example workflow. I have some related questions:

  1. How do I tell gh-action-pypi-publish to publish to test.pypi.org instead of pypi.org.
  2. If published to test.pypi.org, once tested how do I transition it to pypi.org?

Khronos is working on setting up an organization account on pypi.org. Currently we are awaiting a response from pypi.org.

And I have more questions:

  1. How do I build the distribution. The deployment sections you added to the .yml files deploy interface/python/binding/dist but when I build pyktx on my Mac either by building the target in Xcode or manually invoking buildscript.py no dist directory is created. I looked in README.md but it did not help.
  2. The reason I was trying to create/find dist is so I could look at the tar file and get its name so as to add something to one of the .yml files to deploy it. What is its name?

@MarkCallow
Copy link
Collaborator

I figured out the tar file name and now have one source tar deployed into the draft release.

I have just now realized that the version number used in the pyktx files is missing the modifier, -beta1 in the current case. All the other packages have it. Please fix the build so it is included. You can submit a PR and I can cherry-pick the fix into the branch where I'm working on the release.

@MarkCallow
Copy link
Collaborator

@ShukantPal, This is now on the critical path to getting the beta release out so please answer my questions quickly. I just tried to build the package target but still no dist directory.

@MarkCallow
Copy link
Collaborator

I was looking in the source directory not the build directory for the dist directory. Silly me. I still need to know what script builds the packages so I can fix the version number. I'd prefer if you could provide the fix but as you are on holiday I'll understand if you cannot.

@MarkCallow
Copy link
Collaborator

I now see where the version is being set. I need your advice. My concern is if I upload the beta packages labelled as 4.3.0 to PyPI then when I later overwrite them with true 4.3.0 release packages how would pip users be able to upgrade to the new packages. Won't pip and PyPI consider them the same? Therefore I think we need to include the tweak, -beta1, in the package name. If we do so, would someone wishing to install the wrapper have to use 4.3.0-beta1 in the pip command or will 4.3 work.

@ShukantPal
Copy link
Contributor Author

How do I tell gh-action-pypi-publish to publish to test.pypi.org instead of pypi.org.

You can use expression syntax for this: ${{ inputs.test ? ‘<TEST_REGISTRY_URL>’ : ‘<REGISTRY_URL’ }}

If published to test.pypi.org, once tested how do I transition it to pypi.org?

You need to run the publish workflow again, but this time without toggling the test input to true.

How do I build the distribution. The deployment sections you added to the .yml files deploy interface/python/binding/dist but when I build pyktx on my Mac either by building the target in Xcode or manually invoking buildscript.py no dist directory is created. I looked in README.md but it did not help.

Make sure you are running CMake with KTX_FEATURE_PY=ON, and then build the pyktx target.

The reason I was trying to create/find dist is so I could look at the tar file and get its name so as to add something to one of the .yml files to deploy it. What is its name?

I am not sure, but I would suggest using a glob - *.tar.gz. The .zip if you are building on Windows.

Won't pip and PyPI consider them the same? Therefore I think we need to include the tweak, -beta1, in the package name. If we do so, would someone wishing to install the wrapper have to use 4.3.0-beta1 in the pip command or will 4.3 work.

Please read about PyPI versioning schemes

You can append b1,b2,… after the version number to specify a beta release.

@MarkCallow
Copy link
Collaborator

Thanks for your answers.

You can use expression syntax for this: ${{ inputs.test ? ‘<TEST_REGISTRY_URL>’ : ‘<REGISTRY_URL’ }}

This did not answer my question. Where would I do this check of inputs.test? No URL is currently being passed to gh-action-pypi-publish. Does it have an optional input for the URL?

You need to run the publish workflow again, but this time without toggling the test input to true.

How do I remove the packages from test once published to the real registry?

I'll read about the version schemes.

@MarkCallow
Copy link
Collaborator

I changed the byproducts and LIBKTX_VERSION setting in the custom commands to use LIBKTX_VERSION_FULL. The build now produces files with version 4.3.0b1 which is 4.3.0-beta1 normalized per PyPI rules. The tests ran successfully. I've just started a CI build with this change.

@MarkCallow
Copy link
Collaborator

I changed the byproducts ... to use LIBKTX_VERSION_FULL.

Since the actual byproducts use the normalized name as mentioned, I expect they will be built every time a build is run as the CMake custom command will be looking for the unnormalized name. Is there a way to access the normalization function in the Python build system to use it to normalize the byproduct names given to CMake?

@ShukantPal
Copy link
Contributor Author

Is there a way to access the normalization function in the Python build system to use it to normalize the byproduct names given to CMake?

You may want to do the normalization within CMake using string operations like replace.

https://cmake.org/cmake/help/latest/command/string.html#replace

Then, pass that into the Python build directly and use it to find the output.

@ShukantPal
Copy link
Contributor Author

This did not answer my question. Where would I do this check of inputs.test? No URL is currently being passed to gh-action-pypi-publish. Does it have an optional input for the URL?

You can use the following snippet:

with:
    repository-url: https://test.pypi.org/legacy/

here

@MarkCallow
Copy link
Collaborator

@ShukantPal you have a pyktx project on test.pypi.org. How can I take it over and publish to it?

@ShukantPal
Copy link
Contributor Author

ShukantPal commented Dec 19, 2023

@MarkCallow I have deleted my published pyktx project on Test PyPI. You should be able to publish it with your account now.

@MarkCallow
Copy link
Collaborator

MarkCallow commented Dec 23, 2023

@ShukantPal, I'm still waiting for various permissions and account set up for publishing to PyPI. I've gone ahead and made v4.3.0-beta1 release without it. Will fix later. The pyktx assets are deployed in Releases for download. The pyktx docs are deployed at https://github.khronos.org/KTX-Software/pyktx/index.html.

But there is a problem. Anything in the various directories named with _ prefixes are not found. Apparently this is something to do with GitHub's use of Jekyll: https://github.blog/2009-12-29-bypassing-jekyll-on-github-pages/. Is it possible to name these directories differently when generating the docs? If not, I'll have to figure out how to create a .nojekyll file in the built docs root. At present none of the scripts or css is found as they're all in _static.

I've created issue #825 to track this.

@MarkCallow
Copy link
Collaborator

I'm trying to get publishing working. I made changes to the action file to ready it for using trusted publishing and renamed it publish-pyktx.yml. Unfortunately for bureaucratic reasons we can't get things set up for trusted publishing yet so I've changed it to use an API token. I am having great difficulty getting the action to recognize the test-pypi input. I keep getting a startup error:

 Invalid workflow file: .github/workflows/publish-pyktx.yml#L60
The workflow is not valid. .github/workflows/publish-pyktx.yml (Line: 60, Col: 9): Unrecognized named-value: 'test-pypi'. Located at position 1 within expression: test-pypi == false .github/workflows/publish-pyktx.yml (Line: 95, Col: 9): Unrecognized named-value: 'test-pypi'. Located at position 1 within expression: test-pypi

What is wrong? Look at .github/workflows/publish-pyktx.ymlin branchtest_py_publish`.

The error message has not changed since my first attempt where I used if: test-pypi. It is now 'if: ${{ github.event.inputs.test-pypi }} yet the error message has not changed. In between I had if: ${{ inputs.test-pypi }} and got the identical error message.

I will not be able to work on this for nearly a month. I am very disappointed that I could not get this working before going on my break. There is no rush but please investigate while this message is still in your mind.

@MarkCallow
Copy link
Collaborator

What is wrong? Look at .github/workflows/publish-pyktx.ymlin branchtest_py_publish`.

I figured it out. I was not selecting the correct branch when I was dispatching the action. Stupid.

How it gets all the way to attempting to deploy but I get

zipfile.BadZipFile: File is not a zip file
Checking dist/pyktx-4.3.0b1-cp310-cp310-linux_aarch64.whl: 

Please figure out why the .whl file is not apparently a zip file. See here for the build log with the python traceback.

@ShukantPal
Copy link
Contributor Author

ShukantPal commented Jan 12, 2024

pypa/gh-action-pypi-publish#148

I posted a question in the issue thread.

@MarkCallow
Copy link
Collaborator

I resolved the 'not a zip file' issue. The curl command for downloading the assets did not enable redirects so the downloaded files all had zero bytes. I now have the following issues which appear to be with the packages we are creating. Please fix.

Checking dist/pyktx-4.3.0b1-cp310-cp310-linux_aarch64.whl: FAILED
ERROR    `long_description` has syntax errors in markup and would not be        
         rendered on PyPI.                                                      
         No content rendered from RST source.                                   
WARNING  `long_description_content_type` missing. defaulting to `text/x-rst`.   
Checking dist/pyktx-4.3.0b1-cp310-cp310-linux_x86_64.whl: FAILED
ERROR    `long_description` has syntax errors in markup and would not be        
         rendered on PyPI.                                                      
         No content rendered from RST source.                                   
WARNING  `long_description_content_type` missing. defaulting to `text/x-rst`.   
Checking dist/pyktx-4.3.0b1-cp311-cp311-macosx_12_0_x86_64.whl: FAILED
ERROR    `long_description` has syntax errors in markup and would not be        
         rendered on PyPI.                                                      
         No content rendered from RST source.                                   
WARNING  `long_description_content_type` missing. defaulting to `text/x-rst`.   
Checking dist/pyktx-4.3.0b1-cp311-cp311-win_amd64.whl: FAILED
ERROR    `long_description` has syntax errors in markup and would not be        
         rendered on PyPI.                                                      
         No content rendered from RST source.                                   
WARNING  `long_description_content_type` missing. defaulting to `text/x-rst`.   
Checking dist/pyktx-4.3.0b1.tar.gz: PASSED with warnings
WARNING  `long_description_content_type` missing. defaulting to `text/x-rst`.   
WARNING  `long_description` missing.                                            

@MarkCallow
Copy link
Collaborator

@ShukantPal please submit your fixes as a PR against the test_py_publish branch.

I have created scripts to retrieve the release metadata and download assets I think that functionality may be more generally useful. The drawback is that publish_pyktx now has to check out the repo. The scripts and updated .yml are now in the branch. I deleted most of the older logs. Look at this current log to see the errors shown in my previous comment.

@MarkCallow
Copy link
Collaborator

@ShukantPal this is now on the critical path to releasing 4.3.0. Please provide fixes for the descriptions as soon as possible.

MarkCallow pushed a commit that referenced this pull request Jan 28, 2024
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 21, 2024
Add Python bindings, documentation and tests. Add to CI.

Adds a manual Publish step for publishing to PyPl. See KhronosGroup#663 for details.

Fixes KhronosGroup#663.

Co-authored-by: Mark Callow <[email protected]>
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
Add Python bindings, documentation and tests. Add to CI.

Adds a manual Publish step for publishing to PyPl. See KhronosGroup#663 for details.

Fixes KhronosGroup#663.

Co-authored-by: Mark Callow <[email protected]>
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
Add Python bindings, documentation and tests. Add to CI.

Adds a manual Publish step for publishing to PyPl. See KhronosGroup#663 for details.

Fixes KhronosGroup#663.

Co-authored-by: Mark Callow <[email protected]>
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
Add Python bindings, documentation and tests. Add to CI.

Adds a manual Publish step for publishing to PyPl. See KhronosGroup#663 for details.

Fixes KhronosGroup#663.

Co-authored-by: Mark Callow <[email protected]>
KaperD pushed a commit to KaperD/KTX-Software that referenced this pull request Feb 22, 2024
Add Python bindings, documentation and tests. Add to CI.

Adds a manual Publish step for publishing to PyPl. See KhronosGroup#663 for details.

Fixes KhronosGroup#663.

Co-authored-by: Mark Callow <[email protected]>
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