alchem0x2A / py-wdf-reader

Python package for read-only accessing the wdf Raman spectroscopy from Ranishaw WiRE software
MIT License
40 stars 16 forks source link

update DataType #33

Closed expert-main closed 3 years ago

expert-main commented 3 years ago

Faker values are still not gotten accurate. Closing this PR.

alchem0x2A commented 3 years ago

@expert-git I'm ok with the idea to add dummy datatypes since most likely they are not actively used in the Renishaw files. What are the issue with their accuracy?

FYI the build failure are due to some old github action safety issues, should be solved using the latest actions workflow in the master branch now. Feel free to modify the PR and I'll merge it.

expert-main commented 3 years ago

Thanks. You can check this screenshot. Screenshot from 2021-03-19 23-18-33

alchem0x2A commented 3 years ago

I've checked out your PR and without no problem. Maybe you are still running on the PyPI version of the renishawWiRE package.

To simply test, run pip install . on your PR branch again and see if something like from renishawWiRE.types import DataType; print(DataType(24)) gives you desired values.

image
expert-main commented 3 years ago

Screenshot is the result of running PyPI version with my file. I am saying, it's necessary to update DataType. As you see, right now in order to check if there any other errors, I filled some faker values and it worked properly with those values. But I think faker values have to be updated with correct values. I am trying to get define values and can provide you list of accurate values when it's ready.

alchem0x2A commented 3 years ago

I see. that would be very helpful indeed! The renishaw files I have are quite old in their versions. I appreciate if you have help with the new DataTypes etc (and add test files if there's no conflict of interest). Thx!

For now I'll leave the PR open. If you wish to maintain the repo feel free to contact me.

expert-main commented 3 years ago

Cool!

expert-main commented 3 years ago

Could you tell me how you defined DataType? And how I can find that value in wdf file?

alchem0x2A commented 3 years ago

@expert-git The datatype are mainly reverse-engineered properties so for the exact values may need to consult renishaw directly. I simply copied some of the definitions from the gwyddion project (https://sourceforge.net/p/gwyddion/code/HEAD/tree/trunk/gwyddion/modules/file/renishaw.c#l152)

If they are correct then the DataTypes starting from 19 may be the offset with microplate positions. However I could not test those. If you like you can check whether the self.origin_list returned by parsing the file makes sense to you.

expert-main commented 3 years ago

Thanks. While testing some files, I see that DataTypes could be to 26 right now. I am not so sure if it could be greater. In doc you provided, I see endmarker has to always last. What do you think?

expert-main commented 3 years ago

and in line 146, I see frequencey again.

alchem0x2A commented 3 years ago

Yes definitely we should consider adding more DataTypes in order not to break the codes on newer renishaw wdf files. One possible temporary solution may be to use your Faker types and output warnings for unknown datatypes / unittypes, but maybe you can figure out which ones are really necessary. :)

I'm not exactly sure if the meaning of ENDMARKER types in gwyddion, maybe it's just for internal debugging purpose.

It seems in the gwyddion c file, the WDF_DATATYPE_FREQUENCY (line 146) is to replace the deprecated WDF_DATATYPE_SPECTRAL (line 128). So it does not interfere with our current implementation. I'm fine with the new naming.

expert-main commented 3 years ago

good to hear. How about this? This worked in all files that I have.

class DataType(IntEnum):
    Arbitrary = 0
    Frequency = 1
    Intensity = 2
    Spatial_X = 3
    Spatial_Y = 4
    Spatial_Z = 5
    Spatial_R = 6
    Spatial_Theta = 7
    Spatial_Phi = 8
    Temperature = 9
    Pressure = 10
    Time = 11
    Derived = 12
    Polarization = 13
    FocusTrack = 14
    RampRate = 15
    Checksum = 16
    Flags = 17
    ElapsedTime = 18    
    Spectral = 19
    Mp_Well_Spatial_X= 22
    Mp_Well_Spatial_Y = 23
    Mp_LocationIndex = 24
    Mp_WellReference = 25
    EndMarker = 26
expert-main commented 3 years ago

And I see that changed DataType does not effect to all functions which I can use via package such as xdata, spectra, xpos, ypos, print_info(). So I guess we can use this value.