Update README.rst #8
Conversation
syncing with Notion page
| @@ -12,257 +12,291 @@ | |||
| .. |Tests| image:: https://github.com/iterative/dvcx/workflows/Tests/badge.svg | |||
| :target: https://github.com/iterative/dvcx/actions?workflow=Tests | |||
| :alt: Tests | |||
| .. |License| image:: https://img.shields.io/pypi/l/datachain | |||
There was a problem hiding this comment.
|License| need to be removed from the top as well
There was a problem hiding this comment.
@volkfox It looks like it didn't get removed from the top, which is causing build failures
| from datachain.lib.feature import Feature | ||
| from mistralai.client import MistralClient | ||
| from mistralai.models.chat_completion import ChatMessage | ||
| from datachain.lib.dc import Column, DataChain |
There was a problem hiding this comment.
Not ordered and not separated in a usual way. Please run it through a linter.
| from datachain.lib.dc import Column, DataChain | ||
|
|
||
| source = "gs://datachain-demo/chatbot-KiT/" | ||
| PROMPT = "Was this bot dialog successful? Describe the 'result' as 'Yes' or 'No' in a short JSON" |
There was a problem hiding this comment.
please use only capital laters or only lowercase in the variables. not the mix.
| api_key = os.environ["MISTRAL_API_KEY"] | ||
|
|
||
| chain = ( | ||
| DataChain.from_storage(source) |
There was a problem hiding this comment.
Please put the link here, not variable. It would be more visual for users.
| .map( | ||
| mistral_response=lambda file: MistralClient(api_key=api_key) | ||
| .chat( | ||
| model=model, |
There was a problem hiding this comment.
same - get rid of the variable. it would be more visual for users
|
|
||
| .. code:: py | ||
|
|
||
| ds = dc.DataChain("mydataset", version = 1) | ||
| failed_dialogs = chain.filter(Column("mistral_response") == '{"result": "No"}') |
There was a problem hiding this comment.
Is there a way to get rid of JSON-as-text comparison?
|
|
||
| In the example above, the chain resolves to a saved dataset “mydataset”. DataChain datasets are immutable and versioned. A saved dataset version can be used as a data source: | ||
| -> |
There was a problem hiding this comment.
We need to get rid of this -> notation. Please investigate the ways other project show the ouput in the code.
| class CompletionResponseChoice(Feature): | ||
| message: MyChatMessage = MyChatMessage() | ||
|
|
||
| class MistralModel(Feature): |
There was a problem hiding this comment.
Please use pydantic_to_feature() - this should work after #5 is merged
|
|
||
| ## Populate model instances ### | ||
| chain = ( | ||
| DataChain.from_storage(source) |
There was a problem hiding this comment.
no need in source variable - text instead (see above)
| -> | ||
| <class '__main__.MistralModel'> | ||
| <class '__main__.MistralModel'> | ||
| <class '__main__.MistralModel'> |
There was a problem hiding this comment.
It does not look good. How about:
assert isinstance(object, MistralModel)
print(object.dict())
There was a problem hiding this comment.
Also, object is not the best name. obj is ok.
| dc.DataChain("mydataset").export("./", "claude.response") # export summaries | ||
| # pip install mistralai | ||
| # this example requires a free Mistral API key, get yours at https://console.mistral.ai | ||
| # add the key to your shell environment: $ export MISTRAL_API_KEY= your key |
There was a problem hiding this comment.
@dmpetrov @volkfox (probably you discussed this already, just to double check). It feels a bit heavy handed tbh - code-wise (needs some brain power to understand what is going on), also (a bigger concern) - it requires an API key to run (people won't be trying this most likely and in a lot of envs it will be blocked).
|
|
||
| Tutorials | ||
| ------------------ | ||
|
|
||
| * `Computer Vision <examples/computer_vision/fashion_product_images/1-quick-start.ipynb>`_ (try in `Colab <https://colab.research.google.com/github/iterative/dvcx/blob/main/examples/computer_vision/fashion_product_images/1-quick-start.ipynb>`__) |
There was a problem hiding this comment.
Any reason this was dropped?
There was a problem hiding this comment.
Good point. We definitely need these.
|
@volkfox it seems it broke CI. Can you please take a look and fix this? |
syncing with Notion page