Jan Baserba Commodity Classification #2297

regnosys-prod-user · 2023-08-03T18:34:29Z

Linked to Issue #2295 which describes the reasoning behind this PR

Commodity Classification (base product, subproduct, further subproduct)

linux-foundation-easycla · 2023-08-03T18:34:33Z

The committers listed above are authorized under a signed CLA.

✅ login: JanBaserba / name: Jan Baserba (bab9328, 39badd8, 47af72e, 6612e14)
✅ login: PayalKhanna / name: Payal Khanna (48d4db6, 786fde9, 144a411, fa0b36a, 0a29918, da27a12, 196c926, d2defb2, b8cb3cc, daee6cc)
✅ login: rvincentISDA (b628054, 00acfe9, 8048361)
✅ login: davidalk / name: David Al-Kanani (bfdf4f8)
✅ login: eacunaISDA (deec1d4, 3f86524)
✅ login: iansloyan (570c110)

netlify · 2023-08-03T18:34:35Z

✅ Deploy Preview for finos-cdm canceled.

Name	Link
🔨 Latest commit	`daee6cc`
🔍 Latest deploy log	https://app.netlify.com/sites/finos-cdm/deploys/64de2fbe819dce0007f6dab3

chrisisla · 2023-08-04T08:45:45Z

rosetta-source/src/main/rosetta/base-staticdata-asset-common-type.rosetta

-	className string (1..1) <"The name defined by the classification system for a specific attribute in the taxonomy.">
-	value string (1..1) <"The value set by the taxonomu that is specific to the className attribute.">
-	description string (0..1) <"A descuription of the class.">
+    className string (0..1) <"The name defined by the classification system for a specific attribute in the taxonomy">


Could you give a bit more detail as to why the cardinality of className has been relaxed please?

chrisisla · 2023-08-04T08:50:26Z

rosetta-source/src/main/rosetta/base-staticdata-asset-common-type.rosetta

-
-	condition CommodityReferenceFrameworkChoice: <"Requires that either the capacity unit or weather unit is populated.">
-		optional choice capacityUnit, weatherUnit
+    commodityClassification Taxonomy (0..*) <"Identifies a commodity using one or several identification systems (ex.: EMIR Refit Table 4 of the Annex to the Comission Implementing Regulation C(2022) 3588, ISDA 2005 Commodity Definitions SubAnnex A, etc.) with any number of hierarchical layers.">


Okay so commodityClassification replaces commodityBase and subCommodity, as you can now use the new ordinal to define the level of the product i.e. sub, sub sub, etc etc, got it!

chrisisla

The changes make sense to me, I just have the one question on className.

Also it was quite hard to see the code changes due to the formatting (indentation) changes. Were these changes added by Rosetta do you know?

Thanks! Chris

JanBaserba · 2023-08-04T09:15:03Z

Hi Chris, className has been relaxed to (0..1) because, unlike in other usages of Taxonomy, in commodities we may not have the name of the class. For instance for EMIR we could potentially have the name of the layer or not: (className: Commodity Base) classValue: NRGY ordinal: 1 (className: SubProduct) classValue: OILP ordinal: 2 (className: Further SubProduct) classValue: BREN ordinal: 3 But for other classification systems like ISDA Commodity Definitions 2005 SubAnnex A the different layers do not have a name, therefore className does not really make sense. Ex: classValue: Energy ordinal: 1 classValue: Oil ordinal: 2 classValue: Oil-Brent ordinal: 3 Regarding indentation, yes, this is something Rosetta does on its own unfortunately...I removed the indentation changes before contributing but apparently Rosetta introduces them again afterwards...Apologies for that. I hope this answers your questions, but please let me know if you have any further doubts. Thank you very much for the revision, Jan

…

On Fri, Aug 4, 2023 at 10:53 AM Chris ***@***.***> wrote: ***@***.**** commented on this pull request. The changes make sense to me, I just have the one question on className. Also it was quite hard to see the code changes due to the formatting (indentation) changes. Were these changes added by Rosetta do you know? Thanks! Chris — Reply to this email directly, view it on GitHub <#2297 (review)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AYTWB6ESDJ7JK3RD7HYNGN3XTS2A3ANCNFSM6AAAAAA3DFPL7A> . You are receiving this because you were mentioned.Message ID: ***@***.***>

chrisisla

Thanks for the explanation @JanBaserba, I can understand why you have made this item optional now. Before committing the changes I think it would be worth another maintainer just double checking on className being optional, but other than that I'm happy with the changes and have approved this PR.

@finos/cdm-maintainers - please can one of you confirm you can't see any potential issues with className becoming an optional attribute.

…n_Baserba_Subproduct

SimonCockx · 2023-08-08T09:23:00Z

Hi @chrisisla and @JanBaserba. I see you have been experiencing an issue with indentation changes. This is not intended behaviour, so we've internally raised an issue for it. As a workaround, you can turn off any whitespace diffs while reviewing in GitHub, see screenshot below. I hope that helps.

chrisisla · 2023-08-08T10:03:55Z

Thanks for the tip @SimonCockx !

…6bbbe96a6c0072a5f6c3-Jan_Baserba_Subproduct # Conflicts: # RELEASE.md

fixed inconsistent headings

…n_Baserba_Subproduct

iansloyan · 2023-08-09T14:30:37Z

Hi Chris, className has been relaxed to (0..1) because, unlike in other usages of Taxonomy, in commodities we may not have the name of the class. For instance for EMIR we could potentially have the name of the layer or not: (className: Commodity Base) classValue: NRGY ordinal: 1 (className: SubProduct) classValue: OILP ordinal: 2 (className: Further SubProduct) classValue: BREN ordinal: 3 But for other classification systems like ISDA Commodity Definitions 2005 SubAnnex A the different layers do not have a name, therefore className does not really make sense. Ex: classValue: Energy ordinal: 1 classValue: Oil ordinal: 2 classValue: Oil-Brent ordinal: 3 Regarding indentation, yes, this is something Rosetta does on its own unfortunately...I removed the indentation changes before contributing but apparently Rosetta introduces them again afterwards...Apologies for that. I hope this answers your questions, but please let me know if you have any further doubts. Thank you very much for the revision, Jan
…
On Fri, Aug 4, 2023 at 10:53 AM Chris @.> wrote: @.* commented on this pull request. The changes make sense to me, I just have the one question on className. Also it was quite hard to see the code changes due to the formatting (indentation) changes. Were these changes added by Rosetta do you know? Thanks! Chris — Reply to this email directly, view it on GitHub <#2297 (review)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AYTWB6ESDJ7JK3RD7HYNGN3XTS2A3ANCNFSM6AAAAAA3DFPL7A . You are receiving this because you were mentioned.Message ID: @.***>

I am not sure I understand this? Can you share with me the sub annex A examples that support the statement in second half of the comment above?

iansloyan · 2023-08-09T15:39:48Z

@JanBaserba @mgratacos Why would you have a new special attribute in this payout (CommodityPayout) when you can just use productTaxonomy which is inherited from productBase? I cannot approve this change as it creates a new special taxonomy attribute for commodities in the product model. If productTaxonomy does not have the structure you need - then fix it. Do not create a duplicative attribute in Commodity Payout

rvincentISDA · 2023-08-14T10:23:33Z

The data attribute of ProductTaxonomy are not applicable here according to their descriptions. The change is about classifying the underlying commodity product not the trade. The productQualifier would not work with the current design of the qualifications function. Accordingly we only need a commodityClassificiation attribute of type Taxonomy. It enables as classification per any taxonomy, including upcoming ISDA CRP's and other ESMA's. Current commodityBase and subCommodity will be represented via the attributes of Taxonomy. This design has been reviewed and approved with Commodity SMEs on the DPBE (inc. BNPP). We will need to visit in a second phase what additional data conditions is needed for the type Taxonomy.

rvincentISDA · 2023-08-14T10:52:27Z

Point to note was that objective was not to start rationalising out components in the model yet but rather ensure we have elements that support the information needed first.

Updated proposal based upon Ian's input and group discussion.

fixed some typos and clarity

iansloyan · 2023-08-17T13:22:58Z

I can approve this on proviso that some further adjustments to the conditions are made per discussion

Updated conditions.

Conditions update.

Commodity Classification

bab9328

Commodity Classification (base product, subproduct, further subproduct)

regnosys-prod-user requested a review from a team as a code owner August 3, 2023 18:34

regnosys-prod-user added the Rosetta label Aug 3, 2023

chrisisla added Complexity: Minor Change Requires two TA approval Criticality: High Target: Production Category: Modelling Target: Development and removed Target: Production labels Aug 4, 2023

chrisisla reviewed Aug 4, 2023

View reviewed changes

chrisisla previously approved these changes Aug 4, 2023

View reviewed changes

rvincentISDA previously approved these changes Aug 4, 2023

View reviewed changes

PayalKhanna added 2 commits August 4, 2023 15:29

Merge branch 'master' into jbaserba_auth0_61976bbbe96a6c0072a5f6c3-Ja…

48d4db6

…n_Baserba_Subproduct

Update RELEASE.md

786fde9

PayalKhanna dismissed stale reviews from chrisisla and rvincentISDA via 786fde9 August 4, 2023 16:13

rvincentISDA and others added 2 commits August 4, 2023 18:38

Update RELEASE.md

b628054

upadated condition to fix syntax for Python code generator

144a411

davidalk and others added 5 commits August 8, 2023 13:54

work around for python generator bug

bfdf4f8

Merge remote-tracking branch 'origin/master' into jbaserba_auth0_6197…

fa0b36a

…6bbbe96a6c0072a5f6c3-Jan_Baserba_Subproduct # Conflicts: # RELEASE.md

Update RELEASE.md

deec1d4

fixed inconsistent headings

release note updated

0a29918

Update RELEASE.md

3f86524

Merge branch 'master' into jbaserba_auth0_61976bbbe96a6c0072a5f6c3-Ja…

da27a12

…n_Baserba_Subproduct

eacunaISDA assigned iansloyan Aug 9, 2023

JanBaserba and others added 6 commits August 16, 2023 15:42

Commodity Classification Update

39badd8

Updated proposal based upon Ian's input and group discussion.

Update RELEASE.md

196c926

Python workaround

d2defb2

Update RELEASE.md

00acfe9

Python workaround

b8cb3cc

Update RELEASE.md

570c110

fixed some typos and clarity

JanBaserba and others added 3 commits August 17, 2023 13:31

Commodity Classification Conditions Update

47af72e

Updated conditions.

Update RELEASE.md

6612e14

Conditions update.

Update RELEASE.md

8048361

rvincentISDA previously approved these changes Aug 17, 2023

View reviewed changes

Python workaround

daee6cc

PayalKhanna dismissed rvincentISDA’s stale review via daee6cc August 17, 2023 14:33

rvincentISDA approved these changes Aug 17, 2023

View reviewed changes

minesh-s-patel merged commit d88729d into finos:master Aug 18, 2023
7 checks passed

PayalKhanna deleted the jbaserba_auth0_61976bbbe96a6c0072a5f6c3-Jan_Baserba_Subproduct branch August 30, 2023 09:48

hugohills-regnosys mentioned this pull request Aug 30, 2023

Python generator does not implement all Rosetta expressions REGnosys/rosetta-code-generators#246

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Jan Baserba Commodity Classification #2297

Jan Baserba Commodity Classification #2297

regnosys-prod-user commented Aug 3, 2023 •

edited by chrisisla

linux-foundation-easycla bot commented Aug 3, 2023 •

edited

netlify bot commented Aug 3, 2023 •

edited

chrisisla Aug 4, 2023

chrisisla Aug 4, 2023

chrisisla left a comment

JanBaserba commented Aug 4, 2023 via email

chrisisla left a comment

SimonCockx commented Aug 8, 2023

chrisisla commented Aug 8, 2023

iansloyan commented Aug 9, 2023

iansloyan commented Aug 9, 2023

rvincentISDA commented Aug 14, 2023

rvincentISDA commented Aug 14, 2023

iansloyan commented Aug 17, 2023

Jan Baserba Commodity Classification #2297

Jan Baserba Commodity Classification #2297

Conversation

regnosys-prod-user commented Aug 3, 2023 • edited by chrisisla

linux-foundation-easycla bot commented Aug 3, 2023 • edited

netlify bot commented Aug 3, 2023 • edited

✅ Deploy Preview for finos-cdm canceled.

chrisisla Aug 4, 2023

Choose a reason for hiding this comment

chrisisla Aug 4, 2023

Choose a reason for hiding this comment

chrisisla left a comment

Choose a reason for hiding this comment

JanBaserba commented Aug 4, 2023 via email

chrisisla left a comment

Choose a reason for hiding this comment

SimonCockx commented Aug 8, 2023

chrisisla commented Aug 8, 2023

iansloyan commented Aug 9, 2023

iansloyan commented Aug 9, 2023

rvincentISDA commented Aug 14, 2023

rvincentISDA commented Aug 14, 2023

iansloyan commented Aug 17, 2023

regnosys-prod-user commented Aug 3, 2023 •

edited by chrisisla

linux-foundation-easycla bot commented Aug 3, 2023 •

edited

netlify bot commented Aug 3, 2023 •

edited