neo4j-contrib/neomodel

Error in getting started documentation?

curious-broccoli opened this issue · 3 comments

In the documentation for p in germany.inhabitant.all(): is executed.

The listed example never defines Country.inhabitant though, I suspect this is a mistake and should be added (I think by using cypher?).

Indeed, it should be added.

I don't understand what you mean by "cypher".

The definition of Country is here and it would need a OneOrMore relationship inhabitants towards entities of type Person. Do we agree on this or did you have something else in mind?

I agree but I don' know what the proper way is. Because here #420 (comment) you said that there shouldn't be two relationships defined between Country and Person (as it is also mentioned in the documentation). That's why I assumed it should use "cypher". But I am sure you know which is the proper way

Alright, let's get some clarity:

The getting started page, uses an example schema throughout, that is first defined in section "Defining Node Entities and Relationships". So, when the reader hits the section "Relationships", the inhabitants property has not been defined anywhere previous and this can indeed cause confusion.

That bit you have flagged correctly and if you can provide a PR that improves it, that would be fantastic.

The discussion in #420 is slightly different. Indeed, it is best to avoid reciprocal relationships that mean the same thing because you are increasing the amount of relationships that the DBMS has to shift through for a given query, beyond what is necessary.

One possible way to fix that part in the documentation without having to modify the examples extensively is to start the "Relationships" section with something like: "Given a slightly different definition for Person, Country as: [provide the two class definitions with inhabitants for Country and Person without a country], the following code demonstrates working with relationships using neomodel". Feel free to modify this as you see fit, this is just an example.

Hope this is clearer