Skip to content

Conversation

@mattdangerw
Copy link
Member

@mattdangerw mattdangerw commented Nov 29, 2023

Maybe better ways to do this, but a few issues with the version() util we added.

  • For nightly packages we were re-writing __version__ in __init__.py, which would not affect the version() util. So for nightlies keras_nlp.__version__ != keras_nlp.version().
  • We try to keep the local editable install equivalent to the exported paths where possible. But before this change keras_nlp.version is a file module locally, and a function after building the package.

@mattdangerw mattdangerw requested a review from grasskin November 29, 2023 03:30
Copy link
Member

@grasskin grasskin left a comment

Choose a reason for hiding this comment

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

LGTM thank you! Although I think for future-proofing we might want to take a stronger stance on letting the API diverge from the editable version or just disable that option (namex path checks?)

@mattdangerw
Copy link
Member Author

LGTM thank you! Although I think for future-proofing we might want to take a stronger stance on letting the API diverge from the editable version or just disable that option (namex path checks?)

Fair point, I would just really miss the ability to do an editable install. Legitimately much faster if you want to quickly iterate on something outside of unit testing.

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