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

Writing invalid photon hdf5 files #44

Open
Jhsmit opened this issue Apr 18, 2019 · 3 comments
Open

Writing invalid photon hdf5 files #44

Jhsmit opened this issue Apr 18, 2019 · 3 comments

Comments

@Jhsmit
Copy link

Jhsmit commented Apr 18, 2019

Hi I was playing around with the phconvert module and found two minor issues:

I loaded a .ptu file and wanted to save it as hdf5 with the save_photon_hdf5 function. However, I didnt supply some of the required metadata fiels and i got an error when assert_valid_photon_hdf5 was called.

  1. It would be nice if the function would check if the input dict got checked for validity before writing the hdf5-file
  2. When the photon file is not valid, the the close() function is not called so I could not delete the invalid file, unless i restarted the juptyer-notebook kernel. Perhaps with with .. as or try/finally this could be resolved.

In general, very nice module (Y)

@tritemio
Copy link
Contributor

For point 1, it is true the check is done only after writing the HDF5 file to disk. The decision was made to allow a single function to test both phconvert-generated and third-party generated Photon-HDF5 files. We wanted to leave the door open without lock-in for third parties.

I completely understand that when you are debugging your code to create valid Photon-HDF5 files this approach is not very convenient. Ideally, we would like a formal schema so that we can use one of the schema-verification libraries to automatically check for compliance. Instead, Photon-HDF5 evolved organically with hand-written rules (we have a JSON specs file that is close to a schema, but not exactly). So, to avoid code duplication, we ended up with this approach which is the best we could achieve given time and funding constraint.

Point 2, I agree this is an inconvenience most of the times. Sometimes it is convenient to have the file open for exploration. But I agree that would be better to change the default and close the file on error, PR welcome.

PS: phconvert is 100% unfunded and 100% volunteer-based, there are no resources for its development. These types of projects are completely unappreciated in the single-molecule community and don't have big-enough traction to compete for funding. So, realistically, the effort can only be put by few labs which would directly benefit from it, while contributing back to the community.

@Jhsmit
Copy link
Author

Jhsmit commented Apr 26, 2019

Thanks for your reply. I might submit a PR if I can find the time :) I understand the struggle to find time and resources for community projects, especially if they are underappreciated. I've found phconvert already very useful for just reading .ptu and .sm files. So the effort you've put in is much appreciated from my side.

Little bit offtopic:
I've recently used pycorrelate for a publication (BioRxiv), currently I only cite the project by its GitHub url for lack of a DOI. I'm happy to update the reference to a Zenodo DOI or other so you can be credited properly? Getting a DOI on Zenodo is only a matter of linking your GitHub account.

@tritemio
Copy link
Contributor

Thanks!

As for the pycorrelate, glad to hear it is useful to you. I made a new release to trigger a DOI minting:
https://zenodo.org/badge/latestdoi/111135296

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants