sdmccabe / commuter-network Goto Github PK

Shouldn't be too hard, just don't have time right now.

1. They're ugly.
2. They'll present compatibility problems on Windows.

Ideally, rename state_fips_mapping.py to something like definitions.py, then use pathlib throughout to define directories.

I can just copy/paste the scripts and refactor to make, e.g., MA, but it would be better if there were more affordances here.

Traceback (most recent call last):
  File "construct_tract_network.py", line 128, in <module>
    main(args)
  File "construct_tract_network.py", line 95, in main
    nx.write_graphml(G, f"data/derived/{args.output}/tract_commuter_flows.graphml")
  File "</Users/larock/miniconda3/envs/commuter_nets/lib/python3.8/site-packages/decorator.py:decorator-gen-658>", line 2, in write_graphml_lxml
  File "/Users/larock/miniconda3/envs/commuter_nets/lib/python3.8/site-packages/networkx/utils/decorators.py", line 240, in _open_file
    result = func_to_be_decorated(*new_args, **kwargs)
  File "/Users/larock/miniconda3/envs/commuter_nets/lib/python3.8/site-packages/networkx/readwrite/graphml.py", line 150, in write_graphml_lxml
    writer.dump()
  File "/Users/larock/miniconda3/envs/commuter_nets/lib/python3.8/site-packages/networkx/readwrite/graphml.py", line 696, in dump
    self._xml_base.__exit__(None, None, None)
  File "src/lxml/serializer.pxi", line 1347, in lxml.etree.xmlfile.__exit__
  File "src/lxml/serializer.pxi", line 1685, in lxml.etree._IncrementalFileWriter._close
  File "src/lxml/serializer.pxi", line 1691, in lxml.etree._IncrementalFileWriter._handle_error
  File "src/lxml/serializer.pxi", line 199, in lxml.etree._raiseSerialisationError
lxml.etree.SerialisationError: unknown error -1628709398

It seemed to produce valid output, despite the error.

It's revolting and trivial to refactor.

Since this code downloads a bunch of very useful metadata, it would be nice if there were some basic functionality (or even just examples) of how to read that data. It is possible to learn from digging in to the scripts that construct the commuter networks, but maybe there could be a utilities.py that has functions like read_dfs in it to centralize reading in metadata and face those functions to the user.

maybe also size/density.

While we're at it. consider if <18 is a problematic attribute name (GraphML doesn't love it).

The aux files (people working in one state but residing in another) cause a boundary problem. The cleanest way is to have --states define the boundary, i.e., -s ma,me means only people residing and working in MA and ME. This is not the current behavior.

The existing repository provides a series of scripts for downloading the data necessary for constructing the commuter network. It's fine but has major limitations:

1. the CLI is a little finnicky.
2. it's not naturally extensible (what about constructing the adjacency network,
   or the airline flow network, or whatever)?

This doesn't really need to be a command-line script; it's fine to run interactively, since you really only need to run it once to construct the edgelist. What would be better would be something like the following:

1. define something like a SpatialNetworkConstructor class. It has three methods: download the relevant data, construct the relevant network,write the resulting network. (Actually, if construct returned a networkx object, we could just lean on its write_* functions).
2. subclass the SpatialNetworkConstructor with a CommuterNetworkConstructor that implements the existing logic.
3. create other subclasses that do the same thing, for other spatial network representations. Then, they have a consistent API.

The class object can store the relevant configuration information, like spatial resolution, the path the files get downloaded to, thresholding rules, etc.