-
Notifications
You must be signed in to change notification settings - Fork 2
New job #5
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
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This pull request does not contain a valid label. Please add one of the following labels: ['chore', 'fix', 'bugfix', 'bug', 'enhancement', 'feature', 'dependencies', 'documentation']
src/easyscience/Objects/Job/Job.py
Outdated
def calculate_theory(self, *args, **kwargs): | ||
raise NotImplementedError("calculate_theory not implemented") | ||
#pass | ||
# @abstractmethod |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this be deleted?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The summary/info abstract classes will be added later, so I left the code for future reuse.
The calculate_theory
abstract method should not be commented out. Fixed.
|
||
from easyscience.Datasets.xarray import xr # type: ignore |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we not moving away from xarray?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We are, but conversion to scipp in libraries will be done gradually. I could remove the dependency on xarray in EasyScience, though, by deleting type hints.
Virtual classes for the new job and relevant other things.
Also, changes to the lmfit wrapper fixing circular dependencies.