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

The output from load_afq_data is getting pretty busy #81

Closed
richford opened this issue Jul 7, 2021 · 1 comment · Fixed by #80
Closed

The output from load_afq_data is getting pretty busy #81

richford opened this issue Jul 7, 2021 · 1 comment · Fixed by #80
Labels
question Further information is requested

Comments

@richford
Copy link
Collaborator

richford commented Jul 7, 2021

load_afq_data already returns a tuple with up to 8 elements. Depending on how we resolve #78, this might climb up to 9 elements. This is too much and I often forget the order of the results. How should we simplify the returned result:

  • Return a dict with keys X, subjects, groups, etc.
  • Return a namedtuple. See os.stat for an example implementation from the standard library.
  • Leave load_afq_data as is but also provide an AFQDataLoader class with attributes for each return value.

I'm partial to the namedtuple since it can be unpacked like a regular tuple so it would be backwards compatible. But we could also access the fields by name, much like a class instance's attributes. Kind of the best of both.

@arokem WDYT?

@richford richford added the question Further information is requested label Jul 7, 2021
@arokem
Copy link
Collaborator

arokem commented Jul 7, 2021

+1 for the namedtuple

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

Successfully merging a pull request may close this issue.

2 participants