-
Notifications
You must be signed in to change notification settings - Fork 18
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
Writer wishlist #56
Comments
I have a rudimentary exporter nearly complete in pure python using numpy and such at the moment. Needs some adjustments for the XDF to be readable in sigviewer. I was also planning to add a few features not included in the LabRecorder / listed here. I will post here when I am done but I'm going to be delayed until after my MCAT in Mid-Jan. I'll be in touch. After I get the python basic functionality up and running, I'll toss my code onto GitHub and post a link here. I also plan to re-implement it in C or C++ afterward. |
I've also started a prototype, could you upload your not-yet-ready code so I could take a look at it? |
For anyone looking for a python wrapper for an xdf writer, @agricolab has done a great job with pyLieSL, following the example for the LabRecorderCLI here. |
Hi, any progress on this? I'm looking for a way of loading an XDF, modifying it and saving it back. I have checked pyLieSL but it doesn't seem to be able to do that (correct me if I'm wonrg). |
No, it can't. |
Would be great to have this feature |
@rob-luke, you mentioned elsewhere that you (or someone from your group) might be interested in contributing. Is this still true? Do you want to discuss your ideas here? I don't think that anyone else currently has the resources to implement a writer. |
Hi @cbrnr, Indeed, we have written an XDF writer for our needs at our workplace. Sorry for the delay in responding to you, I have taken some time to look at both code bases and look at potential barriers to submitting the code here. I will describe the code we have and the potential barriers to merging. Then we can decide how to proceed together.
Thanks for considering these issues. It would be great to clarify these issues up front. The next step would be to decide on an API, then I could open a PR. Thanks 🙏 |
Regarding the second point, I don't think we intended to implement a streaming writer. This is already done by LSL, and the purpose of an XDF writer here would be to dump data (which is already available in some form TBD) into XDF. Re pydantic, would it be possible to remove the type checks? Or does your writer critically rely on this functionality so it doesn't work without it? |
Unfortunately, it critically relies on this and won't work without it. My primary concern here is that this will change the versions of python that pyxdf would work with. Plus the additional changes to maintenance etc. |
This would require Python 3.7+, which is fine with me. I've never worked with pydantic, but I assume it could be replaced by data classes (and removing all type checks) if we really don't want it? |
Correct. Pydantic handles lots of checking of types and conversion too, to ensure you pass in the right data. So this would need to be all written manually instead. |
OK, so it could be done, and the least effort solution would be to not check and convert at all, but assume that the input is in the desired format. Let's wait what others have to say though. @cboulay @tstenner @agricolab |
Thanks for the contribtution, @rob-luke! My five cents: Although i personally am a fan of type hints and their benefits for static code analysis and readability, their enforcement during runtime is not pythonic, and prevents a lot of things i love in python (like easy monkey patching). I would prefer to drop type checking. As i understand it, you use the type for control flow, don't you? If the type of an argument is relevant for control flow, overloading could be alternative solution which is easy to extend, and would even work for init: E.g. https://github.com/dabeaz/python-cookbook/blob/master/src/9/multiple_dispatch_with_function_annotations/example1.py#L94-L121 |
FYI, LabRecorder was originally a Python app so you can find some inspiration in that old code. |
Thank you @cbrnr, @agricolab, and @cboulay for your considered responses. From the feedback above I understand that adding pydantic as a dependency is not ideal, I understand the reasons for this. Unfortunately with our current workload, we can not commit to rewriting our xdf writer to remove pydantic. Thanks for your consideration. We will keep this on our internal project list, and if our development capacity increases we can revisit this in the future. Thanks for maintaining such a great project, we use it regularly and hope to contribute back when we can. |
Hi @rob-luke , is it possible to find a link to your code anywhere which uses pydantic for other to use in the mean time? |
I have a writer in the works that creates .xdf files that is equivalent to the implementation of the LabRecorder's XDFwriter. Currently, it:
Cython could be included. Should I request a PR when it's done? |
I've started a rough prototype for writer support. Currently, it looks like this:
Goals:
Non-goals:
str
objects to a string stream)The text was updated successfully, but these errors were encountered: