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

Vector class doesn't store the std attribute #140

Closed
vetlewi opened this issue Jul 29, 2020 · 5 comments · Fixed by #141
Closed

Vector class doesn't store the std attribute #140

vetlewi opened this issue Jul 29, 2020 · 5 comments · Fixed by #141
Assignees
Labels
bug Something isn't working invalid This doesn't seem right

Comments

@vetlewi
Copy link
Collaborator

vetlewi commented Jul 29, 2020

When I save a Vector object to file I would expect that all the attributes would be saved such that all attributes would be the same after loading back from file.

@vetlewi vetlewi added bug Something isn't working invalid This doesn't seem right labels Jul 29, 2020
@vetlewi vetlewi self-assigned this Jul 29, 2020
@fzeiser
Copy link
Collaborator

fzeiser commented Jul 29, 2020

Hei! That's indeed a bug. It can be changed for saving in numpy format and plain text, but not directly for mama. Therefore I propose to check if std is set. If if not, proceed as before. If it is, create an additional file with [...]_std; and the corresponding thing with the load function; it might be nice to set a debug message to the logger.

For numpy/plain text one could also just save the additional column; that would be easier.

@vetlewi
Copy link
Collaborator Author

vetlewi commented Jul 29, 2020

I see that this is also missing for the Matrix class.

I'm currently trying to implement a fix.
How about giving the user a warning if trying to save in MaMa format while the std is set?

@vetlewi
Copy link
Collaborator Author

vetlewi commented Jul 29, 2020

For numpy/plain text one could also just save the additional column; that would be easier.

This will break backwards compatibility, is that OK?

@fzeiser
Copy link
Collaborator

fzeiser commented Jul 29, 2020

I don't see how it breaks the compatibility; it's just that the attributes will not be loaded with any version before the bugfix, isn't it?

ompy/ompy/filehandling.py

Lines 280 to 310 in be782f2

def load_txt_2D(path: Union[str, Path]
) -> Tuple[np.ndarray, np.ndarray, np.ndarray]:
mat = np.loadtxt(path)
return mat[1:, 1:], mat[0, 1:], mat[1:, 0]
def load_numpy_1D(path: Union[str, Path]) -> Tuple[np.ndarray, np.ndarray]:
vec = np.load(path)
E = vec[:, 0]
values = vec[:, 1]
return values, E
def save_numpy_1D(values: np.ndarray, E: np.ndarray,
path: Union[str, Path]) -> None:
mat = np.column_stack((E, values))
np.save(path, mat)
def load_txt_1D(path: Union[str, Path]) -> Tuple[np.ndarray, np.ndarray]:
vec = np.loadtxt(path)
E = vec[:, 0]
values = vec[:, 1]
return values, E
def save_txt_1D(values: np.ndarray, E: np.ndarray,
path: Union[str, Path], header='E[keV] values') -> None:
""" E default in keV """
mat = np.column_stack([E, values])
np.savetxt(path, mat, header=header)

@fzeiser
Copy link
Collaborator

fzeiser commented Jul 29, 2020

How about giving the user a warning if trying to save in MaMa format while the std is set?

ok

@vetlewi vetlewi linked a pull request Jul 29, 2020 that will close this issue
fzeiser added a commit that referenced this issue Jul 29, 2020
* Vector.save/load will now also save std
* Added a comment to the release notes on what has been fixed.
* Issue still remains in Matrix class, see #142

Co-authored-by: Vetle Wegner Ingeberg <vetlewi@fys.uio.no>
Co-authored-by: Fabio Zeiser <fzeiser@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working invalid This doesn't seem right
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants