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

describe the need for module-name #1604

Merged
merged 4 commits into from
May 18, 2023
Merged

describe the need for module-name #1604

merged 4 commits into from
May 18, 2023

Conversation

krpatter-intc
Copy link
Contributor

This discussion was pretty critical for me:

#1540

And helped unblock some usages, so proposing that it get added to the readme here

@netlify
Copy link

netlify bot commented May 11, 2023

Deploy Preview for maturin-guide ready!

Name Link
🔨 Latest commit edfa084
🔍 Latest deploy log https://app.netlify.com/sites/maturin-guide/deploys/64663b96591b5e00082f1bf9
😎 Deploy Preview https://deploy-preview-1604--maturin-guide.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@messense
Copy link
Member

messense commented May 13, 2023

Thanks, but I dont think it's the right place to add it, better add it in user guide instead.

(We can keep the readme change)

@krpatter-intc
Copy link
Contributor Author

Thanks, but I dont think it's the right place to add it, better add it in user guide instead.
(We can keep the readme change)

Does that mean this PR is ok and I should also submit one for the user guide? or I should close/reject this PR as you dont want it here in the readme ?

@messense
Copy link
Member

You can push new commits to add it to user guide, and we should mention that you need to also change your #[pymodule] name to _lib_name.

@krpatter-intc
Copy link
Contributor Author

@messense updated with the need to specify the libname in the module

I'd like to do the User Guide as a separate PR. I think this is enough to help with some bare minimums. I'd like to spend a bit more time ramping on user guide layout in general. There's a few spots where I think an example or two would be helpful for newbies like myself.

README.md Outdated
├── README.md
└── src
   └── lib.rs
```

When doing this also be sure to set the module name in our code to match the Cargo.toml's `lib.name`
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's actually the last part of module-name (split by .), lib.name in Cargo.toml doesn't matter when using module-name.

README.md Outdated Show resolved Hide resolved
@messense messense merged commit 2c4573c into PyO3:main May 18, 2023
@krpatter-intc
Copy link
Contributor Author

Thanks @messense !

bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request May 21, 2023
https://build.opensuse.org/request/show/1088131
by user mia + dimstar_suse
- Update to v0.15.3:
  * Fix cross-compile to Apple universal2 in the other platform
    gh#PyO3/maturin#1613
  * Remove serde(flatten) to improve error messages
    gh#PyO3/maturin#1616
  * Allow syn 2.0
    gh#PyO3/maturin#1617
  * describe the need for module-name
    gh#PyO3/maturin#1604
  * Add PEP 517 config_settings support
    gh#PyO3/maturin#1619
  * Keep file mode on Unix
    gh#PyO3/maturin#1622
- Changes in v0.15.2:
  * Remove bors
    gh#PyO3/maturin#1602
  * When determining the python module name, use pyproject.toml
    project.name over Cargo.toml package.name.
    gh#PyO3/maturin#1608
  * Warn on missing python-source contents
    gh#PyO3/maturin#1607
  * Fix rewriting dev-dependencies in sdist
    gh#PyO3/maturin#1610
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.

2 participants