Closed jackschd closed 1 year ago
Check out this pull request on
See visual diffs & provide feedback on Jupyter Notebooks.
Powered by ReviewNB
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:40Z ----------------------------------------------------------------
To update the URLS:
Agreed
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:41Z ----------------------------------------------------------------
You have created the alias for levels at cell 30. Would it be better to use the alias here instead of cube.levels
then? i.e.:
hierarchies["Products"] = { "ProductCategory": levels["ProductCategory"], "ProductName": levels["ProductName"], }
jackschd commented on 2022-12-22T09:36:02Z ----------------------------------------------------------------
Fair point. I'll move that line down to where it's going to be used.
HuifangYeo commented on 2022-12-23T02:29:10Z ----------------------------------------------------------------
It would be great if you could replace all the usage of cube.levels
with levels
instead, since you have created the variable for it.
jackschd commented on 2022-12-23T08:05:39Z ----------------------------------------------------------------
Yep
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:42Z ----------------------------------------------------------------
IMHO, it is better to move this section to the "Lookup measures" section below, so that the reader can have immediate reference to the data structure.
It doesn't seem to connect with the section before or after in this segment.
jackschd commented on 2022-12-22T09:38:58Z ----------------------------------------------------------------
I was trying to keep the datastore stuff all together, and I do say this...
"We'll see how we can use this new type of hierarchy a little later when we discuss lookup measures"
HuifangYeo commented on 2022-12-23T02:20:34Z ----------------------------------------------------------------
Just a suggestion - the placement of the Lookup tables section is a bit disjoint in the flow. I think it might flow better if you put it just after the "Joining tables in database" section and before the "Visualizing the cube". Because effectively, you haven't done any lookup here. You are merely joining the tables. So it should actually be grouped with the Loading data, and probably with a subheader.
jackschd commented on 2022-12-23T08:05:25Z ----------------------------------------------------------------
Agreed
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:42Z ----------------------------------------------------------------
You can also explore sum_product instead
jackschd commented on 2022-12-22T09:47:01Z ----------------------------------------------------------------
That's new to me - I'll add a note to highlight an alternative way of achieving the same thing.
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:43Z ----------------------------------------------------------------
offset=1
looks at the next member instead of the previous member.
jackschd commented on 2022-12-22T09:50:51Z ----------------------------------------------------------------
I'll double-check...
jackschd commented on 2022-12-22T09:57:03Z ----------------------------------------------------------------
You're correct. I'll change this.
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:44Z ----------------------------------------------------------------
There's now an actual function call lookup. Not sure which would be better design-wise.
jackschd commented on 2022-12-22T10:08:19Z ----------------------------------------------------------------
Definitely the lookup one.
jackschd commented on 2022-12-23T08:06:04Z ----------------------------------------------------------------
done
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:45Z ----------------------------------------------------------------
With the latest where behaviour, you can actually reduce the codes into the following:
quantitySold = measures["QuantitySold.SUM"] m["_interval"] = tt.where( { (quantitySold > 0) & (quantitySold < 100): "Quantity less than 100", (quantitySold >= 100) & (quantitySold < 300): "Quantity between 100 and 300", (quantitySold >= 300) & (quantitySold < 500): "Quantity between 300 and 500", (quantitySold >= 500) & (quantitySold < 700): "Quantity between 500 and 700", quantitySold >= 700: "Quantity more than 700", }, default="Unknown", )
jackschd commented on 2022-12-22T10:17:11Z ----------------------------------------------------------------
Nice
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:47Z ----------------------------------------------------------------
If we were to use lookup function, we can have the following:
measures["_transportRate"] = tt.lookup( shippers["Rate"], (shippers["Interval"] == measures["_interval"]) & (shippers["Shipper"] == levels["ShipperName"]), )
Agreed.
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-22T09:14:48Z ----------------------------------------------------------------
You can also introduce the mode="raw"
, it's equivalent to queryResults.reset_index(inplace=True)
.
jackschd commented on 2022-12-22T13:46:41Z ----------------------------------------------------------------
Added.
Fair point. I'll move that line down to where it's going to be used.
View entire conversation on ReviewNB
I was trying to keep the datastore stuff all together, and I do say this...
"We'll see how we can use this new type of hierarchy a little later when we discuss lookup measures"
View entire conversation on ReviewNB
That's new to me - I'll add a note to highlight an alternative way of achieving the same thing.
View entire conversation on ReviewNB
Just a suggestion - the placement of the Lookup tables section is a bit disjoint in the flow. I think it might flow better if you put it just after the "Joining tables in database" section and before the "Visualizing the cube". Because effectively, you haven't done any lookup here. You are merely joining the tables. So it should actually be grouped with the Loading data, and probably with a subheader.
View entire conversation on ReviewNB
It would be great if you could replace all the usage of cube.levels
with levels
instead, since you have created the variable for it.
View entire conversation on ReviewNB
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-23T02:57:15Z ----------------------------------------------------------------
I just discovered the link for the nbviewer is incorrect. It's pointing to the master branch.
The correct one should be: https://nbviewer.org/github/atoti/notebooks/blob/main/notebooks/02-technical-tutorials/introductory-tutorial/main.ipynb
jackschd commented on 2022-12-23T10:28:49Z ----------------------------------------------------------------
Corrected
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-23T02:57:16Z ----------------------------------------------------------------
you can replace cube.levels
with levels
instead.
jackschd commented on 2022-12-23T10:29:01Z ----------------------------------------------------------------
done
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-23T02:57:16Z ----------------------------------------------------------------
I don't see a need for this.
The levels["Interval"]
is a many-to-one mapping to the Orders' shipper.
This measure is also not used anywhere.
jackschd commented on 2022-12-23T10:29:18Z ----------------------------------------------------------------
Removed as unused
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-23T02:57:17Z ----------------------------------------------------------------
The measures["Interval"]
ideally should take into consideration the ProductName
and OrderDate
/ OrderID
levels.
Unless your story actually lumps the transportations across the dates/orders.
jackschd commented on 2022-12-23T10:29:43Z ----------------------------------------------------------------
Re-worked as discussed
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-23T02:57:18Z ----------------------------------------------------------------
Sorry I missed out on this. I think the scope level has to either look at the OrderId or the OrderDate.
If you ignore this, then the intervals will always be greater than 700.
jackschd commented on 2022-12-23T10:30:02Z ----------------------------------------------------------------
Re-worked as discussed
View / edit / reply to this conversation on ReviewNB
HuifangYeo commented on 2022-12-23T02:57:19Z ----------------------------------------------------------------
Sorry, again, I've missed out on the link for the Pnl-explained: https://github.com/atoti/notebooks/blob/main/notebooks/01-use-cases/finance/front-office/pnl-explained/main.ipynb
jackschd commented on 2022-12-23T13:41:11Z ----------------------------------------------------------------
Corrected