-
Notifications
You must be signed in to change notification settings - Fork 35
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
Thoughts on default output directory #54
Comments
The And, yes, the script should use |
So I've made a couple changes to my local version of this script for the CSPP packaging that I'm hoping I can convince you would be good changes are for the examples script here. 😄 How willing are you to break backwards compatibility as long as there is a method for reproducing the same output? If the The other changes I've made have been replacing all |
@djhoese let's go for the route you proposed above. If there's a way to retain backwards compatibility with a default, for instance to |
No, sorry. I completely forgot about this and a lot of other things have come up (including glmtools and CSPP Geo stuff) so I don't think I can make any promises of looking at this this week. |
@deeplycloudy In response to your discussion in #59 about the changes I have planned regarding this issue: the work I'm doing with CSPP Geo has now evolved to the point that I've made my own copy of the make_GLM_grids.py script that I copied from here. I did this because it seemed like backwards compatibility was going to be difficult. With our off-github discussion, this may require even more changes that I was originally thinking. Besides removing/hiding some flags based on our discussion, I've also changed |
Ah, ok, I assumed too much about how you've been working with make_GLM_grids.py in CSSP Geo. I'm open to taking a look at your simpler script, and might be able to spot some other essential ideas that could improve the 'standard' example script. Or maybe we just adopt yours. Let's have that conversation on a separate CSSP needs thread? Or over email is still fine if you prefer that. For this issue in particular, I think there's at least a common need to plumb through (to the output writer) the ability to provide an arbitrary filename template. If you've figured out how to do that part it would be a valuable PR. |
So really what we're talking about is this piece of code in lmatools, right?
I think the easiest way of handling this, with the most flexibility, would be have the user pass in a python format string that gets filled in with a specific set of keys. For example:
Then lmatools does:
Or at least something like this. Now this does look ugly for the filename pattern that is in lmatools right now, but it can have a default and most users probably won't want to customize this. Does GLMtools use a different default filename scheme? |
Overall I like your templating scheme, but what's going on is somewhat more complicated. There's an old lmatools writer interface that is overridden in
and
However, taking your proposed path, perhaps we could revert glmtools to use the lmatools naming infrastructure, and then implement I'd prefer to keep this naming convention as the glmtools default since it's what the Unidata feed expects and matches the standard ABI naming and metadata conventions. I don't care too much how we get there. |
Looking at it more this morning, I think it would be as simple as allowing and using a template in |
I think that should work. After looking at the changes I'm glad you tackled it instead of me. Thanks. One other thing I could see changing that I know you don't want to is changing the default directory to be the current directory. It looks like you did that for the python code, but I understand wanting to keep it the current way for the script so I don't want to push, just mentioning it. It's a backwards incompatible change so now would be the chance. Thanks for handling this. |
You're right, it's better to default to the current directory. If anyone wants to prepend a path they have to copy the whole template string anyway, so in practice this doesn't save anyone much effort while generating surprise directories. The current behavior has been turned into an example, and default is now the current directory in #63. |
Right now the
make_GLM_grids.py
example script outputs to a2019/Oct/13/
formatted directory. This directory isn't modifiable by the user of the script, right? I could update it to allow the user to modify where the files go or so that it always rights to the current directory. This example script seems to be used in a couple places so I'm not sure if users are expected to take it and modify it as needed or if it is the advertised "entry point" to using glmtools from the command line.This output directory only uses the start time of the earliest file. Would it be of interest to glmtools devs to allow for a per-file directory? I could also change the code to do:
Instead of the hardcoded
20
that it has now for the year.Just some thoughts while exploring the code. Thanks for your help.
The text was updated successfully, but these errors were encountered: