logic for get vs generate

[bl-young]

I wanted to clarify the expected logic for get vs generate

getFlowBySector() calls on generateFlowBySector() and then saves the file locally:

flowsa/flowsa/flowby.py

Lines 1520 to 1524 in a45e737

	flowby_generator = (
	lambda x=method, y=external_config_path, z=download_sources_ok:
	cls.generateFlowBySector(x, y, z)
	.to_parquet(f'{settings.fbsoutputpath}{method}.parquet')
	)

but calling generateFlowBySector() directly does not save the file.

I think this logic might be backwards in that genreateFlowBySector() should save the parquet while getFlowBySector() should look first for a local copy before calling generateFlowBySector()

From a development standpoint its nice to be able to call generateFlowBySector() directly to update with new changes

[matthewlchambers]

Technically, getFlowBySector() only generates and saves the file if it doesn't find it locally (since flowby_generator is a function that is only called if no local copy is found), but I see your point. My thought was that some users might not be able to save files locally, so I wanted generateFlowBySector() to return the new FBS and not necessarily have to save it. So, maybe the answer is something like:

# somewhere near the bottom of flowby.FlowBySector.generateFlowBySector()
try:
    fbs.to_parquet(...)
except IOError:  # or whatever the appropriate class of error would be
    log.warning(f'FBS could not be saved due to {IOError}')

perhaps with a custom error class providing a useful message (since _FlowBy already overrides the df.to_parquet() method, this would be doable). That way, if someone can't save the file locally, they can still generate an FBS and work with it in memory. A slight refactoring of the _FlowBy._getFlowBy() code might also be appropriate to accommodate this situation.

[matthewlchambers]

I'm going to mostly stop worrying about this for the time being, except to say that I'd still like generateFlowbySector to actually return the new FBS, rather than returning None or some such. It makes it easier to write scripts where I actually want to generate the FBS from scratch each time.

[bl-young]

It makes it easier to write scripts where I actually want to generate the FBS from scratch each time.

Yes I agree this is how I use it too.

[matthewlchambers]

I've added simple parquet saving to generateFlowBySector(). @bl-young, if you want to take a look, I know it needs some refinement (I think you were adding version numbers and a hash of some sort to the file name). Also, as you brought up in #240, it doesn't include the metadata.

[matthewlchambers]

To answer the original question: we reached the conclusion that generateFlowbysector() should save the file locally (mirroring previous functionality) as well as return the new FBS (for ease of use in scripts).