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

convert bagged scores to float #555

Merged
merged 2 commits into from
Jan 10, 2022
Merged

convert bagged scores to float #555

merged 2 commits into from
Jan 10, 2022

Conversation

frcaud
Copy link
Collaborator

@frcaud frcaud commented Jan 7, 2022

In order to avoid DB error: can't adapt type 'numpy.int64'

@frcaud frcaud changed the title convert bagged scores to float (#554) convert bagged scores to float Jan 7, 2022
@frcaud
Copy link
Collaborator Author

frcaud commented Jan 7, 2022

@rth could you review this change please ?

@codecov
Copy link

codecov bot commented Jan 7, 2022

Codecov Report

Merging #555 (77efecf) into master (dfef37d) will not change coverage.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master     #555   +/-   ##
=======================================
  Coverage   93.51%   93.51%           
=======================================
  Files         103      103           
  Lines        8922     8922           
=======================================
  Hits         8343     8343           
  Misses        579      579           
Impacted Files Coverage Δ
ramp-database/ramp_database/tools/submission.py 97.16% <100.00%> (ø)

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 dfef37d...77efecf. Read the comment docs.

Copy link
Collaborator

@agramfort agramfort left a comment

Choose a reason for hiding this comment

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

I confirm it's a real bug and that fixes it.

@frcaud I think this deserves a changelog entry somewhere.

@rth
Copy link
Collaborator

rth commented Jan 10, 2022

Thanks @frcaud !

@rth rth merged commit dd68a91 into paris-saclay-cds:master Jan 10, 2022
@agramfort
Copy link
Collaborator

@rth can you give us a hand to make a tiny minor release and update the server? I don't think I can do the release myself.

maybe @glemaitre you have a minute otherwise to help here?

🙏 🙏 🙏

@glemaitre
Copy link
Collaborator

I can try to make the release now.

@agramfort
Copy link
Collaborator

agramfort commented Jan 11, 2022 via email

@rth
Copy link
Collaborator

rth commented Jan 11, 2022

As far as I remember there were changes in this release requiring a DB migration. Or did you manage to update without it @glemaitre ?

@glemaitre
Copy link
Collaborator

I made the release, I did not update the server :) If you could give a hand to upgrade the server because I am a bit rusty when it comes to DB migration since I did not do it in the last 2 years :).

@rth
Copy link
Collaborator

rth commented Jan 11, 2022

That works. I'll look into updating the server.

@glemaitre
Copy link
Collaborator

pinging @agramfort since they started the data challenge and he could tell you the worker that are running currently

@rth
Copy link
Collaborator

rth commented Jan 11, 2022

I mean if it's just for this fix the easiest is to make a patch 0.8.2 release just with this change and update the server with that. For the DB migration, there will be some downtime and I would rather not do it if a data challenge tomorrow is depending on it :)

rth pushed a commit that referenced this pull request Jan 11, 2022
@agramfort
Copy link
Collaborator

agramfort commented Jan 11, 2022 via email

@rth
Copy link
Collaborator

rth commented Jan 11, 2022

OK, the 0.8.2 release with this fix is done and the server updated. I restarted the frontend and the schedulers I could find.

@frcaud
Copy link
Collaborator Author

frcaud commented Jan 12, 2022

Great ! Thanks @rth and @glemaitre

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