There are a lot of different timetable formats, each having their own caveats and many of which can currently not be read.
If we have some file that outlines the problems different non-working examples each have, we can prioritize easier, which problems to work on.

When generating the calendar_dates, this may result in service being falsely disabled (possibly only when sundays/holidays use different timetables).

If this is not a problem, add documentation explaining why not.

When adding optional keys, StopPosition and all its usages need to be updated. If we instead use a class, we can easily add default values.
A thing to note is, that the original reason StopPosition was created, is so we can use itertuples, which afaik is faster than iterrows.

The regex used to check if an IFOPT is valid matches a lot more, than what should be accepted as valid IFOPT.
It also does not take the public_transport-value of the node into account. See the osm-wiki article.

Generating the stop_times currently is a big mess.

• Split into multiple methods, maybe outsourcing some of the code to stop_times.py, if that makes sense.
• Add more comments explaining it, if it is still convoluted
• Fix tests/Add new tests

We could add the functionality, to edit a PDFTable/TimeTable in the program, similar to a spreadsheet.

This issue is only here, in case the TODOs are needed for some reason.

This can be done much cleaner and also with better documentation, I guess.

If multiple annotations are active on the same service, but have different defaults, default=off will (probably) take precedence, when adding the annotation dates.

Add comment if this is not a problem.

Title. Two characters of size 8 will have a different threshold for being considered next to each other than two characters of size 20.

Reversing the route and searching in both directions, might lead to improved results, because different StartNodes will be used. This may help, if the last stop of the route can be found easier on OSM than the first one.

The only downside I see, is the worsening of the performance.

• Check, if this would make a difference (probably based on simple_travel_cost_calculation) and how much it would impact performance
• If it does improve the results:
  • Implement it
  • Document how/why
  • Add a new config key, to enable/disable this function. The default should depend on how much the result is improved and how much performance is impacted.
  • Test everything

Colors rock. Use them.

We need to differentiate between terminals that can use color and basically windows though, I guess.
There is also the question whether to use something like colorlog or do it ourselves.

Getting the number of pages here needs error handling.

Add an note to the logger.info(), about how many locations had empty names.

Apparently, we do not have a working handler used in testing.

When we have existing stops, we can calculate the speed between the different nodes and use that, instead of the hardcoded one.

Give a table view over all current annotations added, either at the end, or when a certain input was recognized. Maybe we can reuse the agency table for this.

The html for the display_nodes, should be located in some temp_dir. Its filename should also show the route_id and which view this map is about (all nodes/final route/etc.).
That way, the map can be opened at a later time (or reloaded). If necessary, we can then also add a cli arg to export these maps as well.

Maybe use geopy for the distance calculation, instead of doing it manually and approximately.

• Check the performance impact of doing this.
• If it is not too big:
  • Switch to geopy
  • Add tests
  • Add documentation
• Otherwise:
  • Improve the testing on our own implementation
  • Document why we don't use geopy

When asking, whether a file should be overwritten, adding "overwrite [a]ll/never [o]verwrite" could make sense.
At the same time, this part was originally written, when the output consisted of different .txt files instead of a proper feed. In case this function is only needed for the output feed, there is no need to change it (though we should add a comment explaining the reasoning).

When using --non-interactive, or when it is clear which agency should be used, having to input the agency, once reading is complete is a hassle.
Therefore we should add a cli arg --agency, where the user can supply the agency id to use, or in case a DummyAgency should be created an empty string/-1/something similar.
Need to also raise an error, if the given agency id does not exist in the given agency.txt.

Add aerialway as category for AerialLift and SuspendedCableCar.

When using display_nodes, the zoom is fixed and the location uses the average of all node locations.
Instead, we should find a zoom-level, such that all nodes are visible and move the map to the correct position (which may be the same, as the one currently used).

The StreetCar should implement its own good_values/bad_values.

Search the rows that were dropped, as well as Rows/Columns of type Other, for dates.
We could then suggest these in the annotation part, to make things easier for the user.

I made these notes at some point during (later) development. Need to check if they still apply:

• #83

Not sure what exactly I meant by this...

• 2. Merge consecutive stop columns
  I'm not sure if this applies, because afaik the stop_column has to be the first column of a table.

• 3. Fix no header

Not entirely sure, but at some point the header was not read properly, though this may have very well been an issue with the used options (max_row_distance, etc.)
This may be part of the cleanup_tables step.

• 4. Add fix_split_stopnames/Do not add stopname for search on split stops

I assume, this was about the cleanup_tables step. Not sure about the second part though.

The loglevels INFO/DEBUG are probably used the wrong way.
We could also add more loglevels with different Formatters, for example for simple console output or for verbose output.

When multiple input_files are being used, their IDs may clash. We should at least warn the user about this and abort, if this happens. We could also simply merge the data if this is easily possible, or add a suffix to existing colliding IDs.

The tests for BaseDataClass and BaseContainer are missing from here.

• Check if/how to best test this. The required testing could also be done in the subclasses.
• Implement or remove the testclasses and mark as wontfix.

We could use the gtfs:name/name/?? of the OSM node, instead of what we are doing right now. (How are we even generating the GTFS-stop_name, right now?)

Apparently this does/did not work.

Instead of just dropping rows, that had too much distance to the next, we should add them as "extra rows" to the surrounding tables. In particular they could be used to provide the suggestions for #48).

The help text for the cli arguments is currently a truncated version of the help in the default_config. We could instead read the default configs comments before checking cli arguments.
This has the obvious problem that, if the reading fails, no help can be displayed properly. However, if that happens we have a whole different problem, so in general this should be fine(TM).

Seems like the cache at ~/.cache/pdf2gtfs/osm_cache.tsv is created, even if the cleanup step fails, making subsequent cache reads fail.

There are a few issues with how the node costs are calculated.

1. First and foremost, the function currently used, is arbitrary at best.
   (Currently uses (route_type_score + node_has_this_many_optional_values) ** 2 // 20)

2. We should also change where the node costs are calculated, instead of doing something like this.

In other words, we probably need to

• Move everything node cost related to a single place
• Find a function that has some fixed limit and increases fast for smaller values. (log?)
  It should also take the optional values into account, though it should not increase linearly based on them.
• Document how the function works and why it is a good function
• Test it

The way DayIsActive is used in the calendar.py is pretty ugly. Maybe we should just use a tuple or something.

When comparing two BaseContainer, the entries should be sorted in some way first.

Currently unused routes can exist and are removed. Check if this is true and document why this happens, otherwise document why that can not happen here.

Currently, the best node is simply the one that was selected for most routes.
This seems to work most of the time, but probably selects non-optimal nodes sometimes.
The best route selection should also work on multiple, detached routes.

• Check if the way we currently select the best nodes works properly
• If it does:
  • Document, how and why
  • Recalculate the node scores for the full route.
• Otherwise:
  • Find a better way
  • Document, how/why this works better/at all.

Most of the time, it should be clear, which repeat strategy should be used. We simply need to check for "-" and "/" and set the repeat_strategy to mean in that case.
This requires however, that there are no special symbols with meaning, that were removed during preprocessing. Hence, we should add a new default repeat strategy "autodetect", while keeping the other two, in case autodetection fails.

In case the default cache directory can not be read, use ./.pdf2gtfs_cache or p2g_dir/.cache or something similar. (maybe TempDir with fixed name?)

This way it is immediately clear, which test run which test belongs to.
The name of the tempdir should probably be something like pdf2gtfs_test_{date}_{time}.

Some agencies have resorted to "interesting" ways of using repeat columns (e.g. line 1 of the Leipziger Verkehrsbetriebe).
The problem is twofold:

• First we need to properly detect the columns, as well as the start-/end-stops for that particular repeat interval.
• To properly convert these into normal columns, we basically need to start with the shortest interval and whenever (i * interval) > (k * other_interval) (or something similar), change the route accordingly.

Right now, the first part of this seems to be the bigger problem, while the second part looks like it definitively looks like it could end up looking like a hack.

On the other hand, we could ask the user to provide stop ids and the interval to use between them, similar to how we ask for the agency. This could also be done in conjunction of the above: The detection part is input by the user or some file, while the conversion part is done automatically.

Adding new optional keys should be as easy as

• Add name to OPT_KEYS/OPT_OSM_KEYS
• Add logic to opt_keys_to_int, if the key should impact the node cost
• Add a new field to the associated GTFS object and also updating the required defaults/logic when creating such a GTFS object
• Add a new function called here, before the GTFS feed is written

Currently, this is (also because of #25) not the case.

The cli can be improved in some ways:

• We need a shorthand for frequent commands.
• The help text could be added to the properties instead (#33 ).
• #29
• Some arguments need to be required. (e.g. routetype)
  Also check, if there are any conditionally required arguments.
• Check, which properties would make good arguments as well, but currently are not.

The RowType/ColumnType "detection" is pretty basic right now.
Especially StopColumn detection needs to be checked.

Old tests for the config still exist in test_properties.py.
The ones not testing new properties/functions should be moved to custom_conf

Need to test the IFOPT and wheelchair properly.

For each timetable, check if it is valid before generating the gtfs and add a is_valid property, to easily check this from outside. This should then also be used to raise an error/warning.

The test for the stop_times comparisons fails for some values (see the comments in Line 321-323).