Skip to content

update get dimensionality method.#361

Open
SamTov wants to merge 2 commits into
mainfrom
SamTov_update_get_dimensionality
Open

update get dimensionality method.#361
SamTov wants to merge 2 commits into
mainfrom
SamTov_update_get_dimensionality

Conversation

@SamTov

@SamTov SamTov commented Oct 22, 2021

Copy link
Copy Markdown
Member

Summary of additions and changes

  • Optimised the get dimensionality method.

@SamTov SamTov requested a review from PythonFZ October 22, 2021 09:45

@SamTov SamTov left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Please address comments.

Comment thread mdsuite/utils/meta_functions.py

@PythonFZ PythonFZ left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Test missing

dimensions = 3

return dimensions
return np.where(box != 0, 0, 1, 0).sum()

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Please add a test

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It seems thre has actually been a test for this the whole time. One of the first added.

def test_get_dimensionality(self):
        """
        Test the get_dimensionality method.
        Returns
        -------
        assert that for all choices of dimension array the correct dimension comes out.
        """
        one_d = [1, 0, 0]
        two_d = [1, 1, 0]
        three_d = [1, 1, 1]
        assert get_dimensionality(one_d) == 1
        assert get_dimensionality(two_d) == 2
        assert get_dimensionality(three_d) == 3

@SamTov SamTov changed the base branch from development to main November 25, 2021 12:24
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