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

IO structure : what should happen where? #8

Closed
dbrakenhoff opened this issue Jan 31, 2020 · 3 comments
Closed

IO structure : what should happen where? #8

dbrakenhoff opened this issue Jan 31, 2020 · 3 comments

Comments

@dbrakenhoff
Copy link
Collaborator

dbrakenhoff commented Jan 31, 2020

I believe there are quite some differences between the different from_* methods. Sometimes all the logic is stored in the io.io_*.py files whereas in other cases some of the logic is still performed in the from_*() method. I think this is mostly the case in ObsCollection.

So I'm wondering out loud whether this is something that could be made more uniform?

Proposal
All io methods used for constructing the Obs and ObsCollections should return either:

  • Dataframe (, metadata)
  • List of Obs
  • All io logic to read data and return one of these two options is stored in io_*.py files.

The Obs.from_* methods take the DataFrame and metadata and convert it to an Obs instance in the classmethod
The ObsCollection.from_* methods take the list of Obs and convert it to an ObsCollection instance.

I think making this more uniform will make it easier to write new methods. Thoughts?

Edit: typos

@OnnoEbbens
Copy link
Collaborator

I agree with you completely on the io structure.

Additionally I restructured the to_collection_dict method of the Obs class. Previously this method returned the meta dictionary of every Obs instance. Now it returns a dictionary with all the registered attributes and the values of an Obs instance. I think this works more intuitive when using the ObsCollection.from_list() method. However this may break some stuff. Easiest way to fix it is to call the add_meta_to_df method of an ObsCollection to add data manually from the meta dictionary of an Obs object.

@dbrakenhoff
Copy link
Collaborator Author

That seems to make sense, to only use the registered attributes and leave the Obs metadata attributes alone. This only breaks things where something in metadata was assumed to be in the ObsCollection columns right?

And cool, I'll see if I can get started on restructuring IO methods next week!

@dbrakenhoff
Copy link
Collaborator Author

Closed by #11

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

No branches or pull requests

2 participants