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

App view layout #194

Merged
merged 13 commits into from
Nov 7, 2023
Merged

App view layout #194

merged 13 commits into from
Nov 7, 2023

Conversation

fairchildseb
Copy link
Collaborator

  • Minor updates to Readme
  • Adds .gitignore
  • Adds package-lock.json
  • Adds a new base template for the view shared across pages

image

README.md Outdated

Create and activate a virtual environment

Install dependencies: `pip install -r dev-requirements.txt`
Copy link
Collaborator Author

@fairchildseb fairchildseb Aug 11, 2023

Choose a reason for hiding this comment

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

I don't know how you guys feel about this, it's an attempt to make the setup easier so there's not as much version wrangling. If there's a better way to do this or there's a reason to not do it at all, please let me know and I can remove it along with the dev-requirements.txt

Copy link
Member

Choose a reason for hiding this comment

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

I am not sure how we want to handle this, so I'll defer to @mschrimpf on this

Copy link
Member

Choose a reason for hiding this comment

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

Fine with me. I am worried about keeping this file (along with regular requirements.txt) up to date though. But we can always remove if we are unable to maintain it

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@mschrimpf sorry I'm just seeing this now! I didn't see a requirements.txt file and I didn't want to mess up any prod deploys which is why I took this route. I saw some yamls but I didn't know what was what so I was just trying to get a clean dev install, but yeah you're right ideally there should be one dependency file.

Copy link
Member

Choose a reason for hiding this comment

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

yes agree, your call which one we want to keep

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I guess the real question is, how are you doing your dev installs? Should I be referencing one of the yaml files instead of this requirements.txt file I created? I see environments.yml but that doesn't list all of the dependencies or their versions and I'm not super familiar with all the possible incantations of pip install.

I don't do a lot of python work either so I'm just trying to document my steps to make things easier for the next dev to come along and get things set up. I will defer to you guys 100% in what you want here. I can rename the file requirements.txt I can remove the file entirely. Was just trying to do a little helpful house keeping since I was the latest to get things set up 🙂

Copy link
Member

Choose a reason for hiding this comment

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

Let's do the following:

  1. delete environment.yml
  2. rename dev-requirements.txt -> requirements.txt
  3. remove the in-code requirements of the setup.py and instead read from requirements.txt (2)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Based on this documentation https://packaging.python.org/en/latest/discussions/install-requires-vs-requirements/ I think it might be advisable to leave the list of dependencies in setup.py. Again, I'm not an expert on python dependency management so I'll defer to you on wtv you want here, but I found this while I was trying to figure out the "Right Way" to reference requirements.txt in the setup.py file.

Copy link
Member

Choose a reason for hiding this comment

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

The best would be to move to pyproject.toml but that requires a bit more work.
How about we follow the HuggingFace approach where they specify versions but by default remove them for the install?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@kvfairchild says that migrating all brain-score repos to pyproject.tomls is on her near term todo list so she asked me to punt on this for now.

@fairchildseb
Copy link
Collaborator Author

@mschrimpf @mike-ferguson when you get a chance can you take a look at this please

Copy link
Member

@mike-ferguson mike-ferguson left a comment

Choose a reason for hiding this comment

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

Looks great to me! I left a comment on the README.md file about deferring to Martin on that question, as I do not want to tell you the wrong thing. It's really coming together!

Copy link
Member

@mschrimpf mschrimpf left a comment

Choose a reason for hiding this comment

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

looks good, but please get rid of v2 and update files directly before merging

README.md Outdated

Create and activate a virtual environment

Install dependencies: `pip install -r dev-requirements.txt`
Copy link
Member

Choose a reason for hiding this comment

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

Fine with me. I am worried about keeping this file (along with regular requirements.txt) up to date though. But we can always remove if we are unable to maintain it

benchmarks/templates/benchmarks/base_v2.html Outdated Show resolved Hide resolved
benchmarks/urls.py Outdated Show resolved Hide resolved
benchmarks/views/user.py Outdated Show resolved Hide resolved
@fairchildseb
Copy link
Collaborator Author

fairchildseb commented Oct 11, 2023

@mschrimpf I'd like to suggest that we use the v2 prefix to do the release only because it gives you guys escape hatches while you're making sure everything looks good. I know it'd be nice to just do a clean cutover but in my experience it rarely goes 100% smoothly. If you need to roll back the release after the initial deploy NBD, but if you run the new branch for a while and start developing on it then realize things are missing from the old branch it's harder to look back through old PRs and the git history and try to resurrect stuff after the fact rather than being able to easily run existing code by basically flipping a flag. I of course, will defer to you guys and do as you wish, but I just wanted to throw this out there.

@mschrimpf
Copy link
Member

I would prefer a clean cut, especially because we have a dev site that we can run the new site on for a few days. @kvfairchild?

@kvfairchild
Copy link
Collaborator

I would prefer a clean cut, especially because we have a dev site that we can run the new site on for a few days. @kvfairchild?

@fairchildseb I see the logic in your proposal but I think it's an approach better suited to enterprise platforms than our current setup, so I'm OK with a clean cut as well.

@fairchildseb
Copy link
Collaborator Author

@mike-ferguson would you mind checking out this branch and just confirming that it doesn't break anything you've been working on? I collapsed the views down so there's only the one base.html again and I just don't want to mess anything up.

@fairchildseb fairchildseb merged commit 91234b9 into ui_landing_page Nov 7, 2023
2 checks passed
@fairchildseb fairchildseb deleted the sfairchild/v2-app-view branch November 7, 2023 00:53
mike-ferguson added a commit that referenced this pull request Jan 3, 2024
* Initial Commit with new Bulma system

* Code cleanup

* Cleaned code

* users and urls.py commit

* JS navbar functionality works

* Mobile styling is complete (<768px)

* Rest of styling is done

* Updated links to make sense

* Updated test to account for landing page redesign

* Vision link in test

* Added quest logo

* Removed quest logo, updated text on main sections

* Updated main texts and finalized links

* fixed minor spacing

* added css to commit

* PR comments round 1

* Now extends base.html

* font issue fixed

* removed 'static' from css into inline html (from PR request)

* resizing changes

* navbar and container fixes, permanent gap

* mobile styling done

* in between mobile and tablet styling done

* Final styling, in between sizes done

* Preliminary Login page restyling

* Login page finished

* Change password page redesigned to fit new UI

* Change password-confirm page to new UI

* Sign-Up page redesign done. Css-map removed

* Added new account creation message fix

* Added wrong password banner fade effect

* App view layout (#194)

* Fix styling issues (#203)

* preliminary tutorials

* changed some text

* added text (no text styling) for tutorials into html

* quickstart text and code styling

* deepdive_1 text and code styling

* deepdive_2 text and code styling

* deepdive_3 text and code styling

* added line about package structure for submission

* added submission_structure.png for tutorial

* added https usage for login and reset

* Sfairchild/leaderboard (#201)

* App view component layout

* minor updates

* add dev-requirements.txt

* Updated leaderboard view

* update

* nits

* missing import

* nit rm extra newlines

* moar nits

* fix column widths

* address feedback

* Update benchmarks/views/index.py

* fix table expansion

* fix url

* rm score graph on leaderboard

* rm score graph python

* fix links

* changed URLs to be production-ready (removed preview link)

* add compare route and view (#208)

* add compare route and view

* removed right sections from comapare page

---------

Co-authored-by: Mike Ferguson <[email protected]>
Co-authored-by: Michael Ferguson <[email protected]>

---------

Co-authored-by: SEBASTIAN FAIRCHILD <[email protected]>
Co-authored-by: Mike Ferguson <[email protected]>
Co-authored-by: Michael Ferguson <[email protected]>

* last minute changes on the complete website

* correct db usage

* fixed path to white logo

* fixed https profile -> profile domain would trigger repeat login

---------

Co-authored-by: Martin Schrimpf <[email protected]>
Co-authored-by: Seb <[email protected]>
Co-authored-by: SEBASTIAN FAIRCHILD <[email protected]>
mike-ferguson added a commit that referenced this pull request Jan 11, 2024
* Initial Commit with new Bulma system

* Code cleanup

* Cleaned code

* users and urls.py commit

* JS navbar functionality works

* Mobile styling is complete (<768px)

* Rest of styling is done

* Updated links to make sense

* Updated test to account for landing page redesign

* Vision link in test

* Added quest logo

* Removed quest logo, updated text on main sections

* Updated main texts and finalized links

* fixed minor spacing

* added css to commit

* PR comments round 1

* Now extends base.html

* font issue fixed

* removed 'static' from css into inline html (from PR request)

* resizing changes

* navbar and container fixes, permanent gap

* mobile styling done

* in between mobile and tablet styling done

* Final styling, in between sizes done

* Preliminary Login page restyling

* Login page finished

* Change password page redesigned to fit new UI

* Change password-confirm page to new UI

* Sign-Up page redesign done. Css-map removed

* Added new account creation message fix

* Added wrong password banner fade effect

* App view layout (#194)

* Fix styling issues (#203)

* preliminary tutorials

* changed some text

* added text (no text styling) for tutorials into html

* quickstart text and code styling

* deepdive_1 text and code styling

* deepdive_2 text and code styling

* deepdive_3 text and code styling

* added line about package structure for submission

* added submission_structure.png for tutorial

* added https usage for login and reset

* Sfairchild/leaderboard (#201)

* App view component layout

* minor updates

* add dev-requirements.txt

* Updated leaderboard view

* update

* nits

* missing import

* nit rm extra newlines

* moar nits

* fix column widths

* address feedback

* Update benchmarks/views/index.py

* fix table expansion

* fix url

* rm score graph on leaderboard

* rm score graph python

* fix links

* changed URLs to be production-ready (removed preview link)

* add compare route and view (#208)

* add compare route and view

* removed right sections from comapare page

---------

Co-authored-by: Mike Ferguson <[email protected]>
Co-authored-by: Michael Ferguson <[email protected]>

---------

Co-authored-by: SEBASTIAN FAIRCHILD <[email protected]>
Co-authored-by: Mike Ferguson <[email protected]>
Co-authored-by: Michael Ferguson <[email protected]>

* last minute changes on the complete website

* correct db usage

* fixed path to white logo

* fixed https profile -> profile domain would trigger repeat login

* add community page

* updates

* slack handling, icons

* add oauth table, google oauth flow, google contacts integration

* rm dev shims

* update mailing list and add unsubscribe

* cleanup

* Added line of text to align buttons

* Updated landing page to have link to community

---------

Co-authored-by: Mike Ferguson <[email protected]>
Co-authored-by: Michael Ferguson <[email protected]>
Co-authored-by: Martin Schrimpf <[email protected]>
Co-authored-by: SEBASTIAN FAIRCHILD <[email protected]>
Co-authored-by: Katherine Fairchild <[email protected]>
Co-authored-by: Katherine Fairchild <[email protected]>
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 this pull request may close these issues.

4 participants