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

Use path prefix as default prefix when loading models #28

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

NRHelmi
Copy link
Contributor

@NRHelmi NRHelmi commented Aug 4, 2022

This PR adds the ability to the cli to use models paths as prefix by default like explained below:

  • Default behavior
rai load-models DB_NAME --engine ENGINE_NAME path1/to/model1.rel path2/to/model2.rel

Models will be loaded under path1/to/model1 and path2/to/model2 prefixes.

  • Override default behavior by specifying --prefix
rai load-models DB_NAME --engine ENGINE_NAME path1/to/model1.rel path2/to/model2.rel --prefix ""

Models will be loaded under model1 and model2

@larf311
Copy link

larf311 commented Aug 4, 2022

Need to update the CHANGELOG since this is a breaking change. Or I guess maybe we only need to do that on the next release.

@bradlo
Copy link
Collaborator

bradlo commented Oct 5, 2022

did this get merged? i dont see the branch in the list of remote branches?

@NRHelmi
Copy link
Contributor Author

NRHelmi commented Oct 5, 2022

did this get merged? i dont see the branch in the list of remote branches?

@bradlo this work is not yet merged, the branch is still there
https://github.com/RelationalAI/rai-cli/tree/hnr-hierarchical-structure-models

* Add --prefix option to load-models
* Add get-model-source command
* Use models path as a default prefix (could be overwitten by --prefix flag)
Copy link
Collaborator

Choose a reason for hiding this comment

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

what exactly is the semantic for use of paths in file names? aside from the prefix arg? eg: i assume we canonicalize paths in some way? what does ../foo/../bar/baz/model.rel do? what if its a path from home, eg: ~/bar/baz/model? and what about a fully qualified path? the original idea was to use the basename, and prepend any prefix provided by the prefix arg. simple and unambiguous. what use case does this not handle?

@mbravenboer mbravenboer removed their request for review June 7, 2023 21:39
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.

None yet

3 participants