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

Refactor create_reader into static methods of DeviceReader #25

Open
bruno-f-cruz opened this issue May 9, 2024 · 0 comments · May be fixed by #26
Open

Refactor create_reader into static methods of DeviceReader #25

bruno-f-cruz opened this issue May 9, 2024 · 0 comments · May be fixed by #26

Comments

@bruno-f-cruz
Copy link
Member

bruno-f-cruz commented May 9, 2024

create_reader is currently the single entry point to create a DeviceReader. While easier to learn, it has some problems in that it tries to cramp a lot of functionality into a single method. Additionally, as we find different patterns we might want to use to create new readers, a single method will probably be very difficult to maintain. I propose deprecating it in favor of different methods in the DeviceReader class, what work as constructors of the type. Some that come to mind:

  • .from_file (builds a DeviceReader from a .yml file)
  • .from_url (builds a DeviceReader from a .yml file that must be downloaded from a URL)
  • .from_str (builds a DeviceReader from a string that encodes a .yml file, this will probably also be the convergence point to the previous 2 methods)
  • .from_model (builds a DeviceReader from a Model object)
  • .from_dataset (or from_container (?), need a better name...) (builds a DeviceReader from a target .harp folder where a yml file is expected to be found and data is also expected to be found in a standard logging format).

Finally, with the exception of the last method, there is no way to infer what path _ReaderParams should use. The best we can probably do, is to assume the standard logging structure (yml in the same folder as the registers) and use it. The Reader will crash if the register reader method is called without arguments, but the reader will still work as intended if the path to the file is passed as an argument. In the future, we can think about ways to allow users to override this default behavior to support custom logging containers.

@bruno-f-cruz bruno-f-cruz linked a pull request May 9, 2024 that will close this issue
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 a pull request may close this issue.

1 participant