MLMI2-CSSI / foundry

Simplifying the discovery and usage of machine-learning ready datasets in materials science and chemistry
MIT License
80 stars 16 forks source link

Break `load()` out of Foundry class `__init__` to improve workflow #379

Closed ascourtas closed 8 months ago

ascourtas commented 1 year ago

As a User, I want to be able to know what a foundry function does without reading docs. I want a clear and intuitive workflow for using datasets

There was some miscommunication in #323 (PR #358 ) about how best to disambiguate load() from load_data(). The solution developed entails privatizing load() by loading upon instantiation of the Foundry class, but this disrupts user workflows for loading multiple datasets, using list() or search(), etc.

The new solution is to break load() out of __init__, but instead disambiguate by renaming and potentially further breaking up the functions. For example, one proposed solution is:

Or renaming/refactoring solutions are possible, and require reading through the existing load() and load_data() implementations to decide on next steps

Assumptions:

  1. You are familiar with the contents of load() and load_data() and understand what they currently do
  2. The implementation in #358 is currently in the main branch and needs to change per this ticket
  3. We cannot cut a release unless we either change that or do some git wrangling
  4. You have looked over the Example Workflows defined in our Miro brainstorm

Acceptance Criteria

  1. Given I am a new user of Foundry, I can easily look at the list of functions in my IDE and know exactly what to do to download a dataset, preview the metadata, and use it in a dataframe format
  2. As a User, I can accomplish any of the User Workflows defined without running into friction based on the way downloading, loading, and previewing datasets is defined
### Tasks
- [x ] Review User Workflows (linked above) and discuss user needs and next steps with team
- [ ] Decide on naming solution for `load()` vs `load_data()` and any necessary refactor (see suggested example above)
- [ ] Implement changes 
kjschmidt913 commented 1 year ago
kjschmidt913 commented 1 year ago

Is this sort of getting the boot since we are rethinking class structures?

ascourtas commented 1 year ago

Blocked on class restructure #395