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

Standarize names for virtual files #2755

Closed
seisman opened this issue Oct 18, 2023 · 2 comments · Fixed by #3082
Closed

Standarize names for virtual files #2755

seisman opened this issue Oct 18, 2023 · 2 comments · Fixed by #3082
Labels
maintenance Boring but important stuff for the core devs
Milestone

Comments

@seisman
Copy link
Member

seisman commented Oct 18, 2023

Generally speaking, we're using consistent names for input/output tables/grids. For example, we have

  • the data parameter for most modules that read a table (with a few exceptions)
  • the outfile parameter (and also output_type parameter) for modules that write a table
  • the grid parameter for modules that read a grid
  • the outgrid parameter for modules that write a grid

But for virtual files, w're using different names like infile, fname, grdfile, and csvfile. For example,

with file_context as infile:

with file_context as fname:

) as grdfile, lib.virtualfile_from_data(

) as csvfile:

Things become more complicated when we try to fully get rid of temporary files (#2730) using virtual files. For example, the grdtrack function takes a grid file and a table file as input, and writes to a table file. For the three files:

  • the input grid: grid [consistent with other modules]
  • the input table: points [good name for this module]
  • the output table: outfile [consistent with other modules]

Since we're using virtual files, we also need three virtual file names. In the current grdtrack codes, we use grdfile and csvfile. They're not good names, so I decided to use ingrid, infile and outvfile in PR #2733, but these three names are also bad, especially that outvfile and outfile are very similar and sometimes makes me lost.

I believe we will benefit a lot if we can have consistent (exceptions are allowed) and meaningful names for virtual files. IMO, a good name should make it clear that the name is linked to a virtual file, is a input or output file, and is a grid or a table. I think it makes sense to use a leading v for virtual files.

Maybe names like vingrid, vintable, voutgrid, vouttable (a little too long) or vigrid, vogrid, votbl, vitbl (not readable)?

Thoughts?

@seisman seisman added the question Further information is requested label Oct 18, 2023
@yvonnefroehlich
Copy link
Member

I feel that, in general, a bit longer but readable is better than short and difficult to understand. For me, especially votbl and vitbl are difficult to understand.

I think v for virtual is understandable from the context, and as it is the identical part of all file names, it should be the leading part. For the second part, i.e., input or output, I think we should use in and out. Shortening grid to grd and table to tbl (actually, I often use tab) should be fine. This would lead to vingrd, vintbl, voutgrd, vouttbl. Could vintbl be misunderstood as v int bl? Hm. In this case, maybe it's better to use the longer versions?

@seisman
Copy link
Member Author

seisman commented Oct 29, 2023

I think v for virtual is understandable from the context, and as it is the identical part of all file names, it should be the leading part. For the second part, i.e., input or output, I think we should use in and out. Shortening grid to grd and table to tbl (actually, I often use tab) should be fine. This would lead to vingrd, vintbl, voutgrd, vouttbl

I'm using vingrd, vintbl, voutgrd, vouttbl in PR #2729, and they look not bad.

@seisman seisman added maintenance Boring but important stuff for the core devs and removed question Further information is requested labels Feb 28, 2024
@seisman seisman added this to the 0.12.0 milestone Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
maintenance Boring but important stuff for the core devs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants