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

a new strategy for classes in the data module #49

Closed
aryarm opened this issue Jun 17, 2022 · 1 comment
Closed

a new strategy for classes in the data module #49

aryarm opened this issue Jun 17, 2022 · 1 comment

Comments

@aryarm
Copy link
Member

aryarm commented Jun 17, 2022

Background

Classes in the data module are instantiated with a single parameter: fname for the name of the file containing the data. This seemed like a good idea at first, since most of the classes in the data module were just reading data instead of writing it.

But we're starting to get to the point where we're writing a lot of files now. Unfortunately, the current design of requiring only an fname to instantiate means that, when a function requires a Data instance as input, the function can't really be confident that the data in that object has been loaded.

Proposal

I propose a simple redesign where instead of requiring fname as a parameter to Data.__init__(), we require the data property, itself, as input. Then, we make fname a parameter of the read() and write() methods. The read() method would become a class method and it would return an instance, itself.

Since this would be a breaking change, we should probably implement it sooner rather than later.

Challenges

How would we pass the log instance to the read() function if the read() function became a class method? It would probably have to be a parameter.

@aryarm
Copy link
Member Author

aryarm commented Nov 8, 2022

I'm gonna close this for now. I liked the idea initially, but I'm not sure it would make sense for the GenotypesPLINK class, which has read_samples() and read_variants() methods that only partially load the data in the class

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

1 participant