Skip to content

Conversation

bdunleavy22
Copy link
Contributor

Added get_element_names parameter to get_native_view function

Added `get_element_names` parameter to `get_native_view` function
return self.get_native_view(cube_name=cube_name, view_name=view_name, private=private)

def get_native_view(self, cube_name: str, view_name: str, private=False, **kwargs) -> NativeView:
def get_native_view(self, cube_name: str, view_name: str, private=False, get_element_names: bool = True, **kwargs) -> NativeView:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe rename to skip_element_names with default False.

In most other tm1py functions, we use the skip terminology for optional behaviour changes.

@bdunleavy22
Copy link
Contributor Author

I realized afterwards that I also wanted to change the Element properties, so I made it so that you can set the element properties. If you don't want the element names, you can pass an empty list. Although it's a little less clear how to exclude names, it's more in line with other parts of TM1py when it comes to object properties and it's more flexible

@MariusWirtz
Copy link
Collaborator

I realized afterwards that I also wanted to change the Element properties, so I made it so that you can set the element properties. If you don't want the element names, you can pass an empty list. Although it's a little less clear how to exclude names, it's more in line with other parts of TM1py when it comes to object properties and it's more flexible

I like this. Thank you! please add a test case and we can merge it

Added test cases for retrieving different element properties
Added element_properties argument to the generic "get" function. For MDXViews, it is ignored
@bdunleavy22
Copy link
Contributor Author

I added a test case, and I also added the argument to the generic "get" function (which the test case also tests)

self.assertIsInstance(view, NativeView)
self.assertEqual(view.name, self.native_view_name)
self.assertIsInstance(native_view, NativeView)
self.assertEqual(view, native_view)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think here we're not actually testing that the response is different depending on the element_properties provided.
I think it would make sense to create new, separate test cases for common element_properties values.

If we leave this test as is, it will provide good confirmation that the default behavior hasn't changed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay, that makes sense. In that case I may need some help with creating the tests. I am unsure how these self.assert functions work/how to use them for this pull request

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants