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 GCBM API suggestion #202

Conversation

temitayopelumi
Copy link
Contributor

@temitayopelumi temitayopelumi commented Oct 23, 2022

Description

Hello mentors. In reference to the task given for project two.

I will like to suggest this approach after looking at the FLINT demo last weekend and PR163.

The

  1. The upload endpoint stores files in a flat directory structure, rather than nested directories.
  2. The upload endpoint accepts a category parameter when uploading files, allowing us to support runs like GCBM.Belize.
  3. There is a set_attribute method in the GCBMSimulation class that allows the user set input configuration.

This is still a work in progress. It is not a complete design. But it addresses most of the concerns raised in PR163 and will work well for the current Flint-UI.

I will be glad if I can get the mentor's Feedback on this.

@aornugent @Namyalg @HarshCasper @SanjaySinghRajpoot @Janvi-Thakkar @YashKandalkar

Copy link
Member

@HarshCasper HarshCasper left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi — Thanks a lot for working on this! I would however hold my review on this PR since we wanted to have a plan for the OOP refactor but not actually start with the implementation yet. In case you are selected for Outreachy, we can continue working off this PR (which would need to be rebased with some upcoming new refactors and features). I hope that is fine and I would like to thank you for all the efforts 🙏 🤗

@temitayopelumi temitayopelumi marked this pull request as draft October 26, 2022 09:47
@aornugent
Copy link
Contributor

Hi @temitayopelumi - I think your changes are very aligned with those proposed in #206 and think we should migrate your efforts to the same branch. Thank you for your understanding.

@aornugent aornugent closed this Dec 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants