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.

Sign up for GitHub

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

Jump to bottom

Spectra should be first class data objects #24

Closed
bennyrowland opened this issue Oct 7, 2016 · 0 comments
Closed

Spectra should be first class data objects #24

bennyrowland opened this issue Oct 7, 2016 · 0 comments
Assignees
@lasyasreepada
Labels
enhancement
Milestone
0.3.0

Comments

@bennyrowland
Copy link
Member

bennyrowland commented Oct 7, 2016 •
edited by lasyasreepada

Currently MRSData objects contain FID data and provide various convenience methods and properties, while spectra are bare ndarrays without any additional information. It would be better if the two classes inherited from (let's say) MRSBase giving some core methods, and then provided their own methods on top of those. Calling .spectrum() on an MRSData would return an MRSSpectrum object and calling .fid() on an MRSSpectrum object would return an MRSData object. Then users can work with whichever domain they prefer, without having to go back and forth all the time.
@bennyrowland bennyrowland added this to the 0.3.0 milestone Oct 7, 2016
@lasyasreepada lasyasreepada self-assigned this Oct 14, 2016
lasyasreepada pushed a commit that referenced this issue Nov 14, 2016
Attempts to introduce inheritance to Suspect by creating an MRSBase c…
cfc9f98 
…lass with two subclasses, MRSData and MRSSpectrum. Includes methods in each subclass that convert data to the opposite domain. Delegated adjust_phase() to MRSSpectrum. This structure allows users to work with MRS data in whichever domain they want. Safely deleted mrsdata.py (redundant). Resolves #24

Updated other files as needed to reflect this new inheritance structure
Updated requirements.txt to include pydicom
bennyrowland pushed a commit that referenced this issue Mar 25, 2017
@bennyrowland
Attempts to introduce inheritance to Suspect by creating an MRSBase c…
ec29536 
…lass with two subclasses, MRSData and MRSSpectrum. Includes methods in each subclass that convert data to the opposite domain. Delegated adjust_phase() to MRSSpectrum. This structure allows users to work with MRS data in whichever domain they want. Safely deleted mrsdata.py (redundant). Resolves #24

Updated other files as needed to reflect this new inheritance structure
Updated requirements.txt to include pydicom
bennyrowland added a commit that referenced this issue Mar 25, 2017
@bennyrowland
ENH #24: rebased earlier work to current master
f88bdbe 
 Resuming work on this enhancement after some time, first
 thing is to rebase it onto the current master and get it to
 pass the unit tests, which this commit does
bennyrowland added a commit that referenced this issue Mar 25, 2017
@bennyrowland
ENH #24: added new unit test
84afe31 
Simple new unit test to check that calling spectrum()
and fid() return objects of the correct types
bennyrowland added a commit that referenced this issue Mar 25, 2017
@bennyrowland
ENH #24: improving readability
a63bfd3 
removed some commented code and added docstrings
@bennyrowland bennyrowland mentioned this issue Mar 25, 2017
Merged
bennyrowland added a commit that referenced this issue Mar 25, 2017
@bennyrowland
ENH: make spectra first class objects (#43)
964f246 
* Attempts to introduce inheritance to Suspect by creating an MRSBase class with two subclasses, MRSData and MRSSpectrum. Includes methods in each subclass that convert data to the opposite domain. Delegated adjust_phase() to MRSSpectrum. This structure allows users to work with MRS data in whichever domain they want. Safely deleted mrsdata.py (redundant). Resolves #24
Updated other files as needed to reflect this new inheritance structure
Updated requirements.txt to include pydicom

* Removed an unnecessary __new__ constructor in the MRSData subclass

* Attempts to introduce inheritance to Suspect by creating an MRSBase class with two subclasses, MRSData and MRSSpectrum. Includes methods in each subclass that convert data to the opposite domain. Delegated adjust_phase() to MRSSpectrum. This structure allows users to work with MRS data in whichever domain they want. Safely deleted mrsdata.py (redundant). Resolves #24
Updated other files as needed to reflect this new inheritance structure
Updated requirements.txt to include pydicom

* Removed an unnecessary __new__ constructor in the MRSData subclass

* ENH #24: rebased earlier work to current master

 Resuming work on this enhancement after some time, first
 thing is to rebase it onto the current master and get it to
 pass the unit tests, which this commit does

* ENH #24: added new unit test

Simple new unit test to check that calling spectrum()
and fid() return objects of the correct types

* ENH #24: improving readability

removed some commented code and added docstrings
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@lasyasreepada lasyasreepada

Labels
enhancement
Projects
None yet
Milestone
0.3.0
Development

No branches or pull requests
2 participants
@bennyrowland @lasyasreepada