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

Custom types for wload to be used with produce_or_load #304

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

jonas-schulze
Copy link
Contributor

Follow up to #303#issuecomment-966395495.

This is very much work-in-progress. I just opened the PR so that I won't forget to work on it.

I've had a version that additionally checks wether the user has defined a custom saving method using isempty(methodswith(returntype, _wsave)) but didn't specify returntype to dispatch for wload. Though, this is clearly heading towards the "magic" territory from a user's perspective. Is this desirable?

Given that users might have defined methods for FileIO.load, which does no longer receive all the arguments passed to wsave, how breaking is this?

Also, I didn't test this thoroughly, so even when it's finished, it maybe should simmer a bit before merging.

@Datseris
Copy link
Member

bump @jonas-schulze (I'm waiting on you here, I haven't done a review yet because you haven't labelled it as ready yet)

@jonas-schulze
Copy link
Contributor Author

Hi @Datseris, sorry for the delay. I don't know whether I'll be able to work on this within the next 4 weeks or so; I am trying to push my master thesis over the finish line. Is this blocking in any way? Maybe I can free up some time.

@Datseris
Copy link
Member

No stress, nothing is blocking here. Was just checking if you forgot. Feel free to resume at any point you want. Once you have this ready, simply tag me here and ask for a review!

@Datseris
Copy link
Member

Datseris commented Jul 3, 2023

bump here!

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

Successfully merging this pull request may close these issues.

2 participants