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

add tutorial for setting up dynamically loaded filters #419

Merged
merged 2 commits into from
Apr 28, 2022

Conversation

bendichter
Copy link
Contributor

No description provided.

@bendichter bendichter requested a review from lawrence-mbf April 26, 2022 12:56
@bendichter bendichter self-assigned this Apr 26, 2022
@codecov
Copy link

codecov bot commented Apr 26, 2022

Codecov Report

Merging #419 (51e5bf9) into master (6f5a8c3) will increase coverage by 0.69%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master     #419      +/-   ##
==========================================
+ Coverage   85.31%   86.00%   +0.69%     
==========================================
  Files         125      125              
  Lines        4752     5067     +315     
==========================================
+ Hits         4054     4358     +304     
- Misses        698      709      +11     
Impacted Files Coverage Δ
+tests/+system/DynamicTableTest.m 88.99% <0.00%> (-9.16%) ⬇️
nwbExport.m 64.28% <0.00%> (-7.15%) ⬇️
+types/+util/checkConstraint.m 50.00% <0.00%> (-6.25%) ⬇️
+io/parseReference.m 85.29% <0.00%> (-5.62%) ⬇️
+types/+util/checkDtype.m 71.60% <0.00%> (-5.22%) ⬇️
+misc/getMatnwbDir.m 40.00% <0.00%> (-2.86%) ⬇️
+io/+space/getReadSpace.m 97.43% <0.00%> (-2.57%) ⬇️
+io/writeCompound.m 81.48% <0.00%> (-2.45%) ⬇️
+schemes/exportJson.m 98.14% <0.00%> (-1.86%) ⬇️
+types/+untyped/Set.m 51.32% <0.00%> (-1.83%) ⬇️
... and 88 more

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 6f5a8c3...51e5bf9. Read the comment docs.

@lawrence-mbf
Copy link
Collaborator

lawrence-mbf commented Apr 26, 2022

@bendichter The OSX/Linux mention should be placed above the instructions IMO. I would consider just linking to MATLAB docs for Windows users instead of opening the command prompt.

Can't do suggestions since mlx is technically a zipped binary I think.

@bendichter
Copy link
Contributor Author

bendichter commented Apr 26, 2022

why/how is this different for Windows?

@lawrence-mbf
Copy link
Collaborator

why/how is this different for Windows?

You'd use the system path menus (https://www.howtogeek.com/118594/how-to-edit-your-system-path-for-easy-command-line-access/) for windows instead of opening a command prompt.

This change is permanent on the machine and only requires a MATLAB restart for equivalent changes. I guess for temporary filter usage this script is okay, though I don't know how well it works in cmd/powershell.

@bendichter
Copy link
Contributor Author

bendichter commented Apr 27, 2022

@lawrence-mbf what do you think of these changes?

@lawrence-mbf
Copy link
Collaborator

@lawrence-mbf what do you think of these changes?

You wouldn't set the environment variable in MATLAB but I can append to the documentation later via #421

@bendichter bendichter merged commit 53c2970 into master Apr 28, 2022
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.

2 participants