Add Bookstore example #143

izmalk · 2022-12-01T21:58:54Z

What is the goal of this PR?

As a part of my onboarding, I have prepared yet another example with TypeDB. This one is in Python. Its purpose is to demonstrate how to use Python client to TypeDB.

What are the changes implemented in this PR?

Added a directory commerce/bookstore - with all files of the example. The README.md file contains all necessary documentation for this example.

Added requests.py with search book and search user functions. Minor testing of requests, data generation. Also polished schema and the load_data script.

Moved ordered items generation into load_data.py. Minor fixes

Added search Order functionality. Added tags hierarchy, including reasoning rule. Tags requests - work in progress. Minor fixes.

Added assigning tags of sup-tags rule. Fixed searching tags with reasoning.

typedb-bot · 2022-12-01T21:58:57Z

Added README Added comments Modified load_data.py to create/re-create DB Deleted one of the request examples Changed default DB name

Added info about the new bookstore example into the top-level readme file of the TypeDB Examples repository.

README.md

commerce/bookstore/README.md

commerce/bookstore/python/load_data.py

alexjpwalker · 2022-12-05T16:39:22Z

commerce/bookstore/python/load_data.py

+
+
+# This is a list of imported files with datasets for the DB
+Inputs = [


This feel like something we should type a bit more strongly. (We're quite big on strong typing.) How about introducing an Input class?

I'm not sure I follow - It is not used anywhere outside of loading the data. It does not need to be in the schema. It's basically just a list of .csv file names and the correlated functions to load them.

We can define an abstract base class Input, with subclasses BookInput, UserInput, RatingInput etc.

I have tried to do that in a separate PR here https://github.com/izmalk/typedb-examples/pull/1/files
But I feel like the result is by far worse than the initial variant. I suggest discussing in this separate PR how to proceed. Maybe it's just a problem with my implementation.

I have rebased the PR with classes on the latest version of this repository. So it's ready to be merged

Setup the database - by loading the schema.
If you want I can rename it to something like load_schema?

The previous comment was about the setup() function. Fixed it in typedb/typedb-examples@0655495

alexjpwalker · 2022-12-05T16:44:37Z

commerce/bookstore/python/load_data.py

+]
+
+# This is the main body of this script
+with TypeDB.core_client("localhost:1729") as client:


We have a large number of code comments here. In general, we prefer to minimise code comments, because if the code gets refactored / changed (which we do frequently), then the comments go out of date very rapidly, and people forget about them, because they're just comments - a minor thing on the side when compared against the actual source code itself.

Some of the comments are fairly clear. For example, check the db existence on L264 is obvious from simply reading the code, so we can just delete that comment.

On the other hand, some of the comments are necessary in order to actually make it clear what the line does. We always strive to have our code self-documenting: the code itself should describe what it does, concisely, at a high level.

For example, if check_data() == 0, is not clear about what it means. What are we checking about the data? What does the number "0" mean?

By reading deep into the implementation of check_data, we can figure out all these questions: but the average developer reading the code would be better placed to understand it if the method itself was clear in its naming and in its return value. For example, if has_existing_data() == False (or more tersely, if not has_existing_data()).

We should try to make this code more readable in the sense of "gaining a conceptual understanding, just by reading this method, of how the whole program works." Moreover, we should try to apply this throughout all of our Python code. This may be a fairly major refactor task.

In regards to how many comments:
I feel like we can't give too many comments here. This is not the usual code, but an example. Instead of writing some additional documentation with links to lines of code, we are using comments to annotate almost any important line. Even if it's obvious. That's important because of many reasons, including:

We will have a lot of different users onboarding. Including some of the juniors that might be unfamiliar with even obvious code patterns and functions. Some of them might struggle with English and additional comments in simplified English might help them.

This tutorial's goal is not only to lead newcomers through the process to create a working bookstore. But also to enable them to explore the solution. So the less time and effort they spend to understand the existing solution, the more time and motivation they will have to continue exploring their own ideas and variants. They can spend these additional resources to modify the tutorial a little bit for their intended use case. Or build something of their own. Even the slightest bump on the road can reduce their opinion of our product, motivation to explore it, and/or their time/resources budget for their own exploration. So with that comments even in the most obvious places, I'm trying to do some shortcuts here and there. it's not that these comments will obstruct anything. Please tell me if you think this is the case.

The example itself will. be mostly static. I'm thinking of adding a few addons (like debug logging), maybe even adding an automated test (to test this example with new releases of TypeDB to get a warning even if something will get broken). But in terms of refactoring, I don't think the example will change a lot over time. Even if it will change - comments are very important here and should be double-checked just like the rest of the code.
That's why, as long as these comments are not obstructing anything, I think we should stick to this comment-rich approach in examples.

In regards to function names - yeah, I'll try to re-read and improve what I can. I feel like there is always room for improvement. So it might be an iterative process.

Rereading:

Create a Client, check if DB exists, if not, create it and load data; if it exists, connect to it.

If DB exists, check if it has existing data. If it does, prompt user to reload it; it not, then "setup".

The narrative is better than before - I think everything is now clear except "setup". What does setup do?

Loads schema. I renamed the function in typedb/typedb-examples@0655495

Co-authored-by: Alex Walker <[email protected]>

Fixed a bug with rating calculations - prevented division by zero. Minor refactoring

Added sorting to some requests. Just to show this ability. Fixed a bug with genre tags loading too late to correctly load genre-taging relations. Minor refactoring

Renamed functions to generate queries from templates to generate query

commerce/bookstore/python/load_data.py

kebab-case in the schema

Co-authored-by: Alex Walker <[email protected]>

…ples into izmalk_bookstore_init

Renamed setup() to load_schema()

Implemented show_book function. Refactored some comments.

Created show_user function. Minor comments improvements

super-tag-ownership

Added os.path to fix a problem of starting the load_data.py with any other working dir except python/ directory.

Changed Delivery address dataset to auto-generated addresses for more realistic look.

alexjpwalker · 2023-01-20T13:29:45Z

commerce/bookstore/python/load_data.py

+
+
+def load_data():  # Main data load function
+    with TypeDB.core_client("localhost:1729") as client:  # Establishing connection


Let's store "localhost:1729" in config.py. This is also good practice for readers particularly as their project matures and connects to an external host.

Fixed in typedb/typedb-examples@49cbdd1

alexjpwalker · 2023-01-20T13:31:05Z

commerce/bookstore/python/loaders.py

+        self.verbose = verbose
+
+
+class BookInput(Loader):


When using subclassing we should generally adopt a consistent naming pattern: BookInput is not inherently related to Loader; BookLoader would be.

We should rename these loaders to BookLoader, UserLoader, etc.

Fixed in typedb/typedb-examples@f815079

alexjpwalker · 2023-01-20T13:32:17Z

commerce/bookstore/python/loaders.py

+
+# This is a list of classes to import data. The order of values is important for loading data order.
+# Classes have filenames and corresponding methods to load the parsed data into the TypeDB
+Input_types_list = [GenreInput, GenreHierarchyInput, BookInput, UserInput, RatingInput, OrderInput, BookGenreInput]


As per comment above we should rename this list to loaders .

Since the file is called loaders.py — our reference will be loaders.loaders that way.
That's why I suggest using Loaders_list as a variable name. Just for visual aesthetics.
loaders.Loaders_list
More unique, accurate, and descriptive than loaders.loaders or just loaders.list. But I can see loaders.list as an alternative if you want.

After we discussed it I implemented the loaders.loaders_list variant in typedb/typedb-examples@f815079

alexjpwalker · 2023-01-20T13:35:23Z

Before we proceed to merge this PR, we should ensure that the Bookstore example is tested in Python tests.

Added tests for all main functions. All tests are green.

izmalk · 2023-01-30T10:57:24Z

Before we proceed to merge this PR, we should ensure that the Bookstore example is tested in Python tests.

I have implemented the tests. I hope that's enough for now. typedb/typedb-examples@167562a

alexjpwalker · 2023-01-30T14:27:52Z

commerce/bookstore/python/load_data.py

+            print("Detected DB " + config.db + ". Connecting.")
+            if not has_existing_data():  # Most likely the DB is empty and has no schema
+                print("Attempting to load the schema and data.")
+                if load_schema():  # Schema has been loaded


OK, the calls to core_client look a lot cleaner now that we've introduced the typedb_server_addr config - now let's also simplify the implementation!

In the future, our Docs will contain detailed guidelines on when to create Clients, Sessions and Transactions and what their lifetimes should be. Until then, it's all our internal knowledge. And basically, a Client should, ideally, be created once in the whole lifetime of an application. That's what it's conceptually meant to represent (and it holds onto a single persistent connection with the server under-the-hood; this consumes resource, and we rarely need more than one.)

Currently, we open a client at the start of the main method, but then we open another client in load_schema (and another in has_existing_data, etc.)

Let's refactor all other methods in this file to take in a client parameter. Then, main will create one TypeDB client, and reuse it throughout the entire lifetime of the application.

I'd then do the same thing in requests.py. In this case, create the TypeDB client in the main method. Of course, there is the possibility that the user just exits the application and never uses the client, but this would be an edge case scenario.

Good idea. Fixed in typedb/typedb-examples@090229b

alexjpwalker · 2023-01-30T14:44:33Z

commerce/bookstore/python/test.py

+import config
+from typedb.client import TypeDB, SessionType, TransactionType
+
+class LoadDataTests(TestCase):


See discussion in Discord.

Fixed comments in typedb/typedb-examples@1d54bc4
Also, I added the numbers. Otherwise, it's hard to decipher this comment. Too many non-numbered elements in the list

lolski · 2023-02-10T20:19:35Z

commerce/bookstore/python/todo.md

@@ -0,0 +1,8 @@
+# ToDo list


Is this file meant to be checked in?

Yeah, for the future me or future generations.
I believe It's even mentioned in the readme.

izmalk added 7 commits November 29, 2022 21:42

Commerce bookstore initial commit

9943a15

Added simple requests

4a8b57c

Added requests.py with search book and search user functions. Minor testing of requests, data generation. Also polished schema and the load_data script.

Minor updates after testing

2ac249d

Added order generation/import

d32e1c9

Debugging and polishing data loading

6003880

Moved ordered items generation into load_data.py. Minor fixes

Added orders and tags (WIP)

d079401

Added search Order functionality. Added tags hierarchy, including reasoning rule. Tags requests - work in progress. Minor fixes.

Added super typing

5501562

Added assigning tags of sup-tags rule. Fixed searching tags with reasoning.

izmalk self-assigned this Dec 1, 2022

Added Readme and comments, modified load_data.py

f60c114

Added README Added comments Modified load_data.py to create/re-create DB Deleted one of the request examples Changed default DB name

izmalk marked this pull request as ready for review December 4, 2022 19:29

izmalk requested a review from alexjpwalker as a code owner December 4, 2022 19:29

typedb-bot assigned alexjpwalker Dec 4, 2022

Added info into typedb-examples readme

310eac0

Added info about the new bookstore example into the top-level readme file of the TypeDB Examples repository.

alexjpwalker requested changes Dec 5, 2022

View reviewed changes

izmalk and others added 9 commits December 5, 2022 23:28

Refactoring titles

ae9bc02

Added info for the bookstore example

1ef6b95

Fixed two typos

75040e8

Update commerce/bookstore/README.md

0d9fdec

Co-authored-by: Alex Walker <[email protected]>

Update commerce/bookstore/python/load_data.py

737f59c

Co-authored-by: Alex Walker <[email protected]>

Add comments and minor refactoring

e614f09

Fixed a bug with no rating books

d049e4a

Fixed a bug with rating calculations - prevented division by zero. Minor refactoring

Added sorting and fixed a bug with genre tags

54667a7

Added sorting to some requests. Just to show this ability. Fixed a bug with genre tags loading too late to correctly load genre-taging relations. Minor refactoring

Renamed template functions

5f7a34b

Renamed functions to generate queries from templates to generate query

alexjpwalker reviewed Dec 7, 2022

View reviewed changes

commerce/bookstore/python/load_data.py Outdated Show resolved Hide resolved

izmalk and others added 5 commits December 7, 2022 23:29

Capitalisation refactoring

bac11c1

Refactoring for kebab... case

cf7a8c3

kebab-case in the schema

Update commerce/bookstore/README.md

7a18b0f

Co-authored-by: Alex Walker <[email protected]>

Python capitalisation, 2nd attempt

bc132e9

Merge branch 'izmalk_bookstore_init' of github.com:izmalk/typedb-exam…

926e3a7

…ples into izmalk_bookstore_init

izmalk added 14 commits January 16, 2023 21:01

Fixed path to data directory

8dd078b

Rename setup() function

0655495

Renamed setup() to load_schema()

Loaders verbose and bazel fix experiments

1964bc9

Function and comments refactor

822564e

Implemented show_book function. Refactored some comments.

Finish refactoring functions

f10b9c8

Created show_user function. Minor comments improvements

super-tag rule rename

d32623b

super-tag-ownership

Remove bazel

964b18e

Fixing build without Bazel

277c691

Fixing build without Bazel

d0999e0

Fixing the checkstyle check to build

9c06920

Fixing the checkstyle tests to build 2

15a54fd

Fixing the checkstyle tests to build 3

bf751b0

Add os.path

3d4b5c7

Added os.path to fix a problem of starting the load_data.py with any other working dir except python/ directory.

Refactor delivery address data

ef9fa71

Changed Delivery address dataset to auto-generated addresses for more realistic look.

alexjpwalker requested changes Jan 20, 2023

View reviewed changes

izmalk added 3 commits January 20, 2023 17:36

Store server addr in config.py

49cbdd1

Refactoring input into loader

f815079

Add tests

167562a

Added tests for all main functions. All tests are green.

alexjpwalker requested changes Jan 30, 2023

View reviewed changes

izmalk added 3 commits February 10, 2023 12:36

Create a client only once per app

090229b

Add comments to tests

1d54bc4

izmalk requested a review from alexjpwalker February 10, 2023 14:09

alexjpwalker approved these changes Feb 10, 2023

View reviewed changes

izmalk added 2 commits February 10, 2023 15:33

Revert copyright changes to fix tests

dbda7f6

Minor fixes

f6d4569

izmalk merged commit 85463bc into typedb:master Feb 10, 2023

lolski reviewed Feb 10, 2023

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add Bookstore example #143

Add Bookstore example #143

izmalk commented Dec 1, 2022 •

edited

Loading

typedb-bot commented Dec 1, 2022 •

edited by alexjpwalker

Loading

alexjpwalker Dec 5, 2022

izmalk Dec 5, 2022

alexjpwalker Dec 7, 2022

izmalk Dec 8, 2022

izmalk Dec 11, 2022

izmalk Jan 12, 2023

izmalk Jan 18, 2023 •

edited

Loading

alexjpwalker Dec 5, 2022

izmalk Dec 6, 2022

izmalk Dec 6, 2022

alexjpwalker Jan 12, 2023

izmalk Jan 16, 2023

alexjpwalker Jan 20, 2023

izmalk Jan 20, 2023

alexjpwalker Jan 20, 2023

izmalk Jan 20, 2023

alexjpwalker Jan 20, 2023

izmalk Jan 20, 2023

izmalk Jan 20, 2023

alexjpwalker commented Jan 20, 2023 •

edited

Loading

izmalk commented Jan 30, 2023

alexjpwalker Jan 30, 2023

izmalk Feb 10, 2023

alexjpwalker Jan 30, 2023

izmalk Feb 10, 2023

lolski Feb 10, 2023

izmalk Feb 10, 2023



		# This is a list of imported files with datasets for the DB
		Inputs = [



		def load_data(): # Main data load function
		with TypeDB.core_client("localhost:1729") as client: # Establishing connection

Add Bookstore example #143

Add Bookstore example #143

Conversation

izmalk commented Dec 1, 2022 • edited Loading

What is the goal of this PR?

What are the changes implemented in this PR?

typedb-bot commented Dec 1, 2022 • edited by alexjpwalker Loading

PR Review Checklist

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izmalk Jan 18, 2023 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alexjpwalker commented Jan 20, 2023 • edited Loading

izmalk commented Jan 30, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

izmalk commented Dec 1, 2022 •

edited

Loading

typedb-bot commented Dec 1, 2022 •

edited by alexjpwalker

Loading

izmalk Jan 18, 2023 •

edited

Loading

alexjpwalker commented Jan 20, 2023 •

edited

Loading