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

Code cleanup: remove RawData query module #1454

Closed
3 tasks done
prioux opened this issue Jan 6, 2025 · 5 comments · Fixed by #1460
Closed
3 tasks done

Code cleanup: remove RawData query module #1454

prioux opened this issue Jan 6, 2025 · 5 comments · Fixed by #1460

Comments

@prioux
Copy link
Member

prioux commented Jan 6, 2025

In the early days of CBRAIN we used an ActiveRecord library that wasn't as powerful as the current one.

In order to do some direct column extraction, I created a module that provides these two methods:

result1 = ar_query.raw_first_column(:attribute_name)   # array like [ att, att, att, ... ]
result2 = ar_query.raw_rows([ :atttribute_1, :attribute_2])   # array like [ [att1, att2], [att1, att2], ... ]

These two methods are completely unnecessary anymore, because ActiveRecord has the method pluck() which does almost exactly the same thing.

This issue is about:

  1. removing completely the module file BrainPortal/lib/cbrain_extensions/active_record_extensions/relation_extensions/raw_data.rb
  2. searching the entire codebase and replacing the calls to raw_first_column() and raw_rows() with pluck() instead.

Some code adjustment might be necessary given the raw_xxx() methods can be invoked without any arguments, unlike pluck().

Checklist over the next few days:

@MontrealSergiy
Copy link
Contributor

I think first step 2 should be done, both for core and external plugins, then some time latter step 1.

Otherwise, few important tool will cease working ( including civet and DICOM converters)

MontrealSergiy added a commit to MontrealSergiy/cbrain that referenced this issue Jan 13, 2025
MontrealSergiy added a commit to MontrealSergiy/cbrain that referenced this issue Jan 13, 2025
MontrealSergiy added a commit to MontrealSergiy/cbrain that referenced this issue Jan 13, 2025
@prioux
Copy link
Member Author

prioux commented Jan 13, 2025

Right. I didn't think about the plugins.

So yes, first we do 2. We commit and deploy. Once we know for sure the CBRAIN codebase and the plugins don't use the raw_X() methods anymore, we perform 1.

@prioux
Copy link
Member Author

prioux commented Jan 13, 2025

Just go ahead!

@MontrealSergiy MontrealSergiy linked a pull request Jan 13, 2025 that will close this issue
@prioux prioux removed a link to a pull request Jan 14, 2025
prioux pushed a commit that referenced this issue Jan 14, 2025
* Code cleanup: replace RawData with plucking stage 1 #1454
@prioux
Copy link
Member Author

prioux commented Jan 14, 2025

Important!! You missed a few files in the CBRAIN code base:

BrainPortal/lib/tasks/cbrain_plugins.rake
BrainPortal/app/views/tasks/_tasks_display.html.erb
BrainPortal/app/views/userfiles/_quality_control_panel.html.erb
BrainPortal/app/views/users/_users_table.html.erb

For your next pull request, make sure to update these files AND remove the raw_data.rb module

@prioux
Copy link
Member Author

prioux commented Jan 14, 2025

Also, make your PR with master as the base branch (make sure you start with the latest master too)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants