-
Notifications
You must be signed in to change notification settings - Fork 15
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
Switch from printing to logging messages #41
Comments
I agree - this is a good idea but let's hold off until the merge. |
I think I have a rather clean way of handling the logging. I just have a couple of questions:
|
|
Okay, I can incorporate it into the timing logger then.
Fair enough! I can use exceptions and
I can stick with a freeform text file for now, and then we can revisit if you want to leverage more of the features logging provides. I'll open a draft PR today and you can see what I'm thinking. |
One thing that I'm stuck on is the |
Ah - that's a little obscure. I have a wrapper function, addmemprofiling. If memory_profiler is install, then memprofile is True then that wraps functions in a call to profile(), a function from memory_profiler that gives detailed memory usage information (more detailed that what the tide_util function logjam provides). I have an unresolved issue with extremely large files on linux. It seems to never actually release memory when objects are deleted - I think the term is "high water mark allocation". On the mac, the VMEM happily goes up and down as I clear out arrays. On linux, it only ever goes up, and gets absurd (57GB when processing HCP resting state data). Adding all the profiling stuff was part of an (abandoned) effort to get to the bottom of that. I still intend to fix it someday. I don't think having it in there is harming anything for the most part - memory_profiler maintains its own file. I suppose we could change the logic - instead of doing one or the other, always log to to logmem, but if memory_profiler exists, also add the profiling wrapper, which will write to its own file (instead of NOT logging to logmem if memory_profiler is there). |
Is your feature request related to a problem? Please describe.
Printing messages throughout the workflow doesn't let you control what level of information you see as a user. If you switch to using
logging
, then you can define the log level of each message, so devs can run with debugging-level messages, while users can run with just warnings and errors.Describe the solution you'd like
Throughout the library, swap
print
statements out withlogging
.Describe alternatives you've considered
None, really. Logging would be helpful, and the
logging
library is a built-in, so there are no additional dependencies.Additional context
None
EDIT: I'm happy to help with this once
dev
is merged in. Working on it before then would probably introduce a bunch of merge conflicts.The text was updated successfully, but these errors were encountered: