Updated fbf packages

[kgraaf]

removed offending code that prevented bootstrap components upgraded and pinned necessary versions of pandas and shapely

Two packages had to be pinned to specific (old versions) so that the environment could be safely added to:

1. Pandas:

   Newer versions of Pandas do not appear to work correctly with queuepool. In particular, how that package forms SQL query objects needs to be updated. Hopefully the solution here is to either send Igor a issue or to migrate to a different connection pooling library

2. Shapely:

   Shapely changed how looping over geometries works in the newer version and Pingrid relies on the old functionality. It is unclear to me whether using the .geoms property has the same behavior as the "built in" iterator the old version had. As we may migrate way from Pingrid and we're not going to upgrade it right now and possibly waste the work or effort.

The dash bootstrap components in v1 dropped support for hide_arrow and innerClassName. I suspect the reason why the former was used is that trying to consistently style the tooltip arrows is extremely difficult and I couldn't figure out. You can get the upwards pointing arrow to work but now the other directions. The latter removal is fairly easy to remedy with CSS selectors. However, the display is still subtly different.

Before:

After:

(Note that the lack of the logos is not related to this change, I just don't have that set up in my dev environment, but it isn't a regression)

As a brief aside, the tooltips that hover over the table are not actually bootstrap tooltips, they're built in functionality of dash_table with an entirely different structure. As such, trying to get style consistency between the two is extremely difficult and fraught. I tried but couldn't succeed. Also, while I thought that dash_table allowed you to set a class for them this turns out to not be true after all, so even styling them is difficult (and fragile, as I would guess because I don't see any indication that the specific CSS classes they happen to use are necessarily intended to be stable across versions of the library)

[kgraaf]

• make control bar and table tooltips consistent in styling (too complicated so decided to recreate previous status quo)
• add notes on packages that had to be pinned

[aaron-kaplan]

After doing some reading, I think the disadvantages of consolidated metadata outweigh the benefits for us. The benefit is supposed to be that it makes open_zarr faster, but for the datasets we're using open_zarr is already essentially instantaneous. The disadvantage is the potential for inconsistency. The .zmetadata file is a centralized collection of duplicate copies of the metadata from the individual variables that make up the dataset. In a setup where the zarr store will be updated periodically, there is a risk that we update metadata in a variable but fail to update the consolidated metadata, either because we forget, or because the program crashes between steps. This is yet another potential cause of the frustrating "why isn't the new forecast appearing" problem, and I would rather avoid it.

@kgraaf please add consolidated=False to all calls to open_zarr, so that it will continue to work with non-consolidated zarr stores as it did with xarray 0.19.

• use open_zarr with consolidated=False

[kgraaf]

@aaron-kaplan this should be ready to go

[aaron-kaplan]

Whenever we change something that affects the appearance of the application, let's get in the habit of attaching a screenshot to the PR. Although in this case it might be tricky to get a screenshot of a tooltip?

[kgraaf]

Just added screenshots to the original comment

[aaron-kaplan]

If you haven't already, please test on the data in /data/aaron/fbf, to verify that consolidated=False does in fact suppress the consolidated metadata warning.

[kgraaf]

If you haven't already, please test on the data in /data/aaron/fbf, to verify that consolidated=False does in fact suppress the consolidated metadata warning.

I believe I already did but I will again just to be sure

[aaron-kaplan]

There was a todo item "add notes on packages that had to be pinned." You checked that item off, but I can't find the notes. Did you forget to push them?

[kgraaf]

There was a todo item "add notes on packages that had to be pinned." You checked that item off, but I can't find the notes. Did you forget to push them?

I added them to the first comment on this PR

[aaron-kaplan]

I added them to the first comment on this PR

Ah, right. Please summarize them as comments in environment.yml where the pinning happens.

[kgraaf]

Ok cool. Will do

[kgraaf]

Wait, I got this wrong. It's a bug with rasterio not Conda. Also, incidentally, GH is saying you requested changes but I can't even see what this is referring to

[aaron-kaplan]

GH is saying you requested changes but I can't even see what this is referring to

That's attached to my comment of 3:17pm asking where the explanation of the pinned packages was.

[kgraaf]

Oh ok cool. The UI was just a little confusing

[aaron-kaplan]

Remember to run the tests before you merge. conda install pytest and then python -m pytest, in the fbfmaproom directory and conda environment.

[kgraaf]

OK Thanks for the reminder!

[kgraaf]

Ok tests all ran successfully but I did have to prefix your command with CONFIG=fbfmaproom-sample.yaml. I assume that's right. I also updated the package import to not give the deprecation warnings.

[aaron-kaplan]

I did have to prefix your command with CONFIG=fbfmaproom-sample.yaml.

Yes, right.

Ordinarily I like to deploy changes as soon as they're merged, but I think this might be a particularly sensitive time for the project, so I will check with Dan before we deploy.

[aaron-kaplan]

Oops, forgot to update environment_macos.yaml. I have an old macbook I got from Jeff for things like this, will take care of it in a follow-up PR.

[kgraaf]

OK thank you! I've been wondering how we should do that since I have no means to

[aaron-kaplan]

More precisely, new versions of pandas.read_sql and pandas.read_sql_query no longer accept a DBAPI object as the connection. Apparently it worked by coincidence, not by design. A change made last July made it an error, but people complained, so they changed it to a warning.

The solution appears to be to use psycopg2 via SQLAlchemy rather than directly. I think that's a good move regardless; it's the community standard.

[aaron-kaplan]

And once we have SQLAlchemy as a dependency, there's no cost to also using its QueuePool instead of queuepool, which should solve the problem of applications hanging when the database server restarts.