tickets/SP-2082: Support getting obsloctac visits from the pre-night sim archive

[ehneilsen]

I've reworked the start and end times of the obsloctap function to match what @rhiannonlynne proposed to Will:

you call rubin_sim.sim_archive.fetch_obslactap_visits() at any time,
and we return the visits starting from the next sunset after the current time through the 2nd sunrise after that.
So, if you happened to call during the night (after sunset, before sunrise), we will NOT return any visits from the current night, but will return the visits from the next two nights.

[rhiannonlynne]

A comment and question that is optional, but the error message should be modified (I gave a suggestion) so that people don't think their misspelling means they should try installing lsst.resources.

[rhiannonlynne]

I have recently found that
", ".join([colname for colname in dbcols])
should work for this in a one-liner.
Worth testing.

[ehneilsen]

I was just moving what was there, without thinking about it too hard.
What's the list comprehension for? Why not just:

", ".join(dbcols)

[rhiannonlynne]

Oh true .. dbcols is already a list here. (was remembering this from a different situation, where the columns were coming back from a query to the consdb schema, and so was not a list).

[rhiannonlynne]

I think this error message may need some tweaking.
By far the most common error people are going to make is to try to use a mis-spelled version of the filename, or try to access a database that isn't where they think it is.
Most of these people (who include not-us) will not be trying to load a file from an S3 bucket, but may not be savvy enough to realize that lsst.resources is irrelevant to what they need.
The error message also slightly conflates two different issues - having lsst.resources versus the file existing anywhere (since it could still be a bad or inaccessible uri even with lsst.resources)

Can we tell if the uri is a local file vs. a uri that should be read with lsst.resources? And then give an error message that is as simple as "FileNotFoundError".
If not, then we could indicate why we were trying to use lsst.resources ..

try:
            from lsst.resources import ResourcePath
except ModuleNotFoundError:
            raise RuntimeError(f"Could not find {db_con} as a local file; please install lsst.resources in order to read remote file")
# then try to read the remote file
 with ResourcePath(db_con).as_local() as local_db_path:
                with closing(sqlite3.connect(local_db_path.ospath)) as con:
                    sim_data = pd.read_sql(query, con).to_records(index=False)

[ehneilsen]

The error messages lists the two issues as possibilities because it can arise from either cause.

Can we tell if the uri is a local file vs. a uri that should be read with lsst.resources?

I started to write code to do this, and concluded that it was more effort than it was worth, thinking that the user would know whether the file they are trying to read was local so diagnostics to be more specific wouldn't be particularly useful.

A test that is usually right shouldn't be that hard, though: if db_con is a string that does not start with a URI "scheme" designation, its probably intended to be a file name. But, in principle this can fail, because linux files names can have colons in them, so really any URI can be a path to a local file. Naming a file or directory http: or s3: is a foolish and confusing thing to do, but it can happen. It can also fail if support is added for new schemes.

[rhiannonlynne]

Yeah, ok - I'm ok if we can't decide whether it's local or not ourselves, but the thing to do is to make it clear that we're using lsst.resources for reading remote files only (rather than someone who didn't know what lsst.resources is thinking that they needed to install it to read their misspelled database filename). Because the general users for rubin_sim may very well not know what lsst.resources is or how to install it.

[rhiannonlynne]

is sim_metadata a pandas dataframe? would it make sense to make it one, so that the section below becomes a query of the dataframe?

[ehneilsen]

sim_metadata is a list of dicts. It can be trivially turned into a dataframe, but doing that doesn't really simplify things here. Some of the elements we are concerned with are lists or dicts. Yes, a cell in a pandas DataFrame can be a dict or a list, but dealing with them is awkward, so using a DataFrame ends up making the code harder to read.

[ehneilsen]

I've reworked it so it only tries using ResourcePath if db_con is either not a str, or it can be parsed with pythons URL parser to get a valid scheme. So, it will only mention ResourcePath if it actually needs it.