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

Add pylance #25093

Merged
merged 28 commits into from
Feb 7, 2024
Merged

Add pylance #25093

merged 28 commits into from
Feb 7, 2024

Conversation

yhori991
Copy link
Contributor

@yhori991 yhori991 commented Jan 21, 2024

Checklist

  • Title of this PR is meaningful: e.g. "Adding my_nifty_package", not "updated meta.yaml".
  • License file is packaged (see here for an example).
  • Source is from official source.
  • Package does not vendor other packages. (If a package uses the source of another package, they should be separate packages or the licenses of all packages need to be packaged).
  • If static libraries are linked in, the license of the static library is packaged.
  • Package does not ship static libraries. If static libraries are needed, follow CFEP-18.
  • Build number is 0.
  • A tarball (url) rather than a repo (e.g. git_url) is used in your recipe (see here for more details).
  • GitHub users listed in the maintainer section have posted a comment confirming they are willing to be listed there.
  • When in trouble, please check our knowledge base documentation before pinging a team.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pylance) and found it was in an excellent condition.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I wanted to let you know that I linted all conda-recipes in your PR (recipes/pylance) and found some lint.

Here's what I've got...

For recipes/pylance:

  • Non noarch packages should have python requirement without any version constraints.
  • Non noarch packages should have python requirement without any version constraints.

@conda-forge-webservices
Copy link

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipes/pylance) and found it was in an excellent condition.

@yhori991
Copy link
Contributor Author

I'm happy to be listed as the maintainer of this recipe 👍

@yhori991
Copy link
Contributor Author

@conda-forge/staged-recipes this PR is ready for review.
I could not make a practical win-64 build but still, it is hopefully helpful for those who need this package on conda ecosystem. lancedb/lance#636

Copy link
Contributor

To help direct your pull request to the best reviewers, please mention a topic-specifc team if your recipe matches any of the following: conda-forge/help-c-cpp, conda-forge/help-cdts, conda-forge/help-go, conda-forge/help-java, conda-forge/help-julia, conda-forge/help-nodejs, conda-forge/help-perl, conda-forge/help-python, conda-forge/help-python-c, conda-forge/help-r, or conda-forge/help-ruby. Thanks!

@yhori991
Copy link
Contributor Author

Thanks for the comments and reviews @xhochy.

Aside from win-64 issues, the current change looks much cleaner and better.
The error log is in the L1538 of this log down below.

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=862707&view=logs&j=240f1fee-52bc-5498-a14a-8361bde76ba0&t=7c0f8eae-6d6f-51bf-636f-73a1a7fb1bc4&l=1538

It is a protoc error that can be seen neither in OSX nor Linux host environment, which is saying format.proto was not found. This error message is mysterious to my eyes since the original repo does not have format.proto from the beginning 🤔

@yhori991 yhori991 requested a review from xhochy January 23, 2024 16:14
@yhori991
Copy link
Contributor Author

I've just noticed several code issues and made two updates too.

  • fixed the project home URL
  • strictly align maturin build parameters of UNIX host and Windows host

The win-64 build issue is still not resolved but it might be now easier to narrow down the cause.

@yhori991
Copy link
Contributor Author

Luckily for us, bumping the package version up to 0.9.10 solved the win-64 build issue it seems.

@yhori991
Copy link
Contributor Author

yhori991 commented Feb 3, 2024

Bumping the version number to v0.9.12 breaks the linux-64 build.

From the CI log, there seems to be an overlinking issue on xz library.

conda_build.exceptions.OverLinkingError: overlinking check failed
[" ERROR (pylance,lib/python3.8/site-packages/lance/lance.abi3.so): .. but ['conda-forge/linux-64::xz==5.2.6=h166bdaf_0'] not in reqs/run, (i.e. it is overlinking) (likely) or a missing dependency (less likely)"]

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=869517&view=logs&j=6f142865-96c3-535c-b7ea-873d86b887bd&t=22b0682d-ab9e-55d7-9c79-49f3c3ba4823&l=1455

@yhori991
Copy link
Contributor Author

yhori991 commented Feb 3, 2024

I'll first check how it goes on v0.9.11 and narrow down the cause of the issue.

@yhori991
Copy link
Contributor Author

yhori991 commented Feb 3, 2024

On the surface level, there are no significant changes on Cargo.toml files that seem to be relevant to the linking issue above. I'll think about adding xz as a runtime dependency to see if it works (there's still a chance that it might be truly a missing dependency issue)

@yhori991
Copy link
Contributor Author

yhori991 commented Feb 3, 2024

It is building xz2 in here, which is a binding to liblzma (xz utils).

https://dev.azure.com/conda-forge/feedstock-builds/_build/results?buildId=870007&view=logs&j=6f142865-96c3-535c-b7ea-873d86b887bd&t=22b0682d-ab9e-55d7-9c79-49f3c3ba4823&l=1292

It might be legitimate to add xz as a dependency.

@xhochy xhochy merged commit 8ade71c into conda-forge:main Feb 7, 2024
5 checks passed
@yhori991 yhori991 deleted the add-pylance branch February 7, 2024 15:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

2 participants