Skip to content

Conversation

fchollet
Copy link
Collaborator

This PR demonstrates how public API export would work:

  • There's a decorator that does both public API symbol registration and optional serializable registration (all public API symbols should be serializable, as a general rule).
  • There's a build script that uses namex at build time to generate a package that only includes public symbols.

To create the package, simply run python3 build.py instead of python3 -m build. The resulting artifacts can then be collected from dist/ as usual.

TODO: export all public API symbols. Right for we only export one (TransformerEncoder) for demonstration purposes.

build.py Outdated
namex.generate_api_files(package, code_directory="src", verbose=True)

# Make sure to export the __version__ string
from keras_nlp.src import __version__
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Is there a way to escape the lint error here?

Copy link
Member

Choose a reason for hiding this comment

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

You can add # noqa to the end of the line.

Or maybe # noqa (ignore import lint error) to be a little more readable.

@mattdangerw mattdangerw self-requested a review February 13, 2023 20:02
@fchollet
Copy link
Collaborator Author

I updated the PR -- how do you want to proceed? Should we just add the export calls everywhere in this PR and merge? Should we merge first then start a call for contribution to do the conversion? We don't it to have it finished until we do the next PyPI release.

setup.py Outdated
"tensorflow-text; platform_system != 'Darwin'",
],
extras_require={
"build": ["namex"],
Copy link
Member

Choose a reason for hiding this comment

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

I think we can just move this to requirements.txt

@mattdangerw
Copy link
Member

mattdangerw commented Mar 7, 2023

@fchollet when I try to run this naively, e.g.

gh pr checkout 747
pip install namex
python build.py

I get a weird infinite recursion. E.g.

...processing keras_nlp/src/src/src/src/src/src/src/src/src/src/src/src/src/src/src/tokenizers/__init__.py

And eventually a max recursion depth error. Am I doing something wrong in setup?

Copy link
Member

@mattdangerw mattdangerw left a comment

Choose a reason for hiding this comment

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

LGTM!

@fchollet fchollet merged commit de866a1 into keras-team:master Mar 9, 2023
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