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

Update read_header function to support EFM data Files #42

Merged
merged 2 commits into from
May 1, 2021

Conversation

agualdron
Copy link
Contributor

EFM datafiles are generated by EFM140 Software which is part of the Gamry Software modules.
When I tried to parse an EFM .DTA file, no curves were detected by the parser.
The reason is because in this files the curve data starts after a "EFMCURVE" tag.

So, I add the letters "EFM" to the regex on the read_header function.
Now the EFM curve is detected and everything works fine.

Here is an example EFM file:
GEN_EFM_#3.DTA

EFM datafiles are generated by EFM140 Software.
When I tried to parse an EFM .DTA file, no curves were detected by the parser.
The reason is because in this files the curve data starts after a "EFMCURVE" tag.

Son I add the letters "EFM" to the regex on the read_header function. 
Now the EFM  curve is detected and everything works fine.

Here is an example EFM file:
https://drive.google.com/file/d/1OcGX-0FHC6UC8n0f6ryxLELOEIW78agm
@bcliang
Copy link
Owner

bcliang commented Apr 27, 2021

Thanks for the PR.

  • Tests passed. As it's just a change to read_header I don't think a new test is necessary, so we should be all good to go.
  • Before I merge the PR, please update the CHANGELOG as part of the PR. I made suggested changes in the attached
    CHANGELOG.md
  • I don't have experience with EFM140 software. Any particular processing required to interpret the data? Should we create a subclass to handle EFM curves?

@bcliang bcliang added the enhancement New feature or request label Apr 27, 2021
agualdron added a commit to agualdron/gamry-parser that referenced this pull request May 1, 2021
Added: [bcliang#42](bcliang#42) Update read_header function to support EFM data Files
Add: Update read_header function to support EFM140 data Files
@agualdron
Copy link
Contributor Author

Thanks for your review...

  • The EFM files are quite simple and have some similarities with the LPR (Linear Polarization Resistance) files. I don't think a new test is necessary for the EFM files. LPR files are working well from the beginning, and don't have tests either.
  • I already made a "commit" to branch "patch-1" with the updated changelog as you requested.
  • For the use cases I'm working with this file, no further processing is required to parse the data. The data as returning by your amazing parser is just excellent.

Thank you very much for this software, and for allowing me to contribute at least a little bit.

@bcliang
Copy link
Owner

bcliang commented May 1, 2021

Thanks for your help, PR merged!

For your workflow, do you need the released sbin/wheel from pypi, or are you able to manage just building from source? I can cut new release today if needed. Otherwise, it might sit in master without a new release published to pypi until mid-May. I have been working on peak detection + area calculation with cyclic voltammetry experiments and was hoping to include that in the next release. Unfortunately, it's still a week or two out.

@bcliang bcliang merged commit 33c7e2b into bcliang:master May 1, 2021
@agualdron
Copy link
Contributor Author

Thanks!

I can build from source by now, it is not an issue....
I will look forward to the next release, and hope I can contribute more on this project...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants