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

September grab bag #819

Merged
merged 19 commits into from Oct 6, 2020
Merged

September grab bag #819

merged 19 commits into from Oct 6, 2020

Conversation

rwblair
Copy link
Collaborator

@rwblair rwblair commented Sep 21, 2020

@adelavega
Copy link
Collaborator

Looks like the date in the analysis listing is only date-month, why not day? I think just classic MM-DD-YY would be best probably

Minor comment about the last modified date, it's really close to the (i) hover thing, which makes them seem related.
I would say either mom the date into the body, or put the last modified date into a "Tag" to visually separate them.
It made me expect that the (i) was going to tell me about the last modified date when its just general info.

By Tag I like the way "PASSED" is displayed on the Status tab.

We could also just put the last modified date on the Status tab if there's more room there.

@codecov-commenter
Copy link

Codecov Report

Merging #819 into master will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #819      +/-   ##
==========================================
+ Coverage   83.23%   83.31%   +0.07%     
==========================================
  Files          63       63              
  Lines        2899     2912      +13     
==========================================
+ Hits         2413     2426      +13     
  Misses        486      486              
Impacted Files Coverage Δ
neuroscout/resources/predictor.py 90.62% <100.00%> (+1.46%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dd9f290...8e4e1d9. Read the comment docs.

@rwblair
Copy link
Collaborator Author

rwblair commented Sep 22, 2020

Right now for date formatting I have dd-mm-yyyy

The world is a big scary place and we Americans are not making it any easier:
https://en.wikipedia.org/wiki/Date_format_by_country

@adelavega
Copy link
Collaborator

loaded on server for testing. so far looks good to me, although tests are failing.

@rwblair can you tag all the issues addressed in the initial comment?

@adelavega
Copy link
Collaborator

Tests fixed, if you can just list those issues and I can verify all is well I think we're ready to merge

@rwblair
Copy link
Collaborator Author

rwblair commented Oct 6, 2020

Just the note that is in progress in .8 project, #813 and potentially #619 when task selecting is implemented for predictor uploads. There were a few bugs that didn't have issues that were fixed (collection list failing with no uploaded predictors, api call to neurovault failing)

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.

User generated predictors not showing up in predictor list Remove promise resolution from constructors
3 participants