Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Raise an exeption if the (not entirely implemented) analytical versio… #777

Closed
wants to merge 1 commit into from

Conversation

finnmayhew
Copy link
Contributor

…n of the effective NSI coupling matrix is accessed

…n of the effective NSI coupling matrix is accessed
@finnmayhew finnmayhew requested review from 1ofgp, LeanderFischer and VeronikaPalusova and removed request for 1ofgp July 17, 2024 18:51
@finnmayhew
Copy link
Contributor Author

Hm, running this function should probably not be in the unit tests since it's not implemented.

@@ -388,6 +388,9 @@ def eps_matrix(self):
def eps_matrix_analytical(self):
"""Effective NSI coupling matrix calculated analytically."""
# Analytical relations. These are wrong right now! #FIXME

raise NotImplementedError("The analytical veresion of the effective NSI coupling matrix is not yet implemented.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

there is a typo here

@LeanderFischer
Copy link
Collaborator

LeanderFischer commented Jul 18, 2024

Hm, running this function should probably not be in the unit tests since it's not implemented.

Yeah, totally, but even a step before, why is this function (body) even there, if it's apparently not implemented? I suggest do get rid of it altogether, if there is no value in keeping the body and/or the function at all, no?

Edit: Ah ok, this is in the process of being implemented? I suggest do leave incomplete code out of the master branch, work on it in a fork/branch until it is ready, and then get it in. Feel free to leave the empty function head and the NotImplementedError, but then take it out of the test of course 🙏

@LeanderFischer
Copy link
Collaborator

Solved through #786

@finnmayhew finnmayhew deleted the finnmayhew-patch-3 branch August 12, 2024 19:54
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