Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add option for alternate name of hwy_data directory #595

Open
jteresco opened this issue Jul 20, 2023 · 21 comments
Open

Add option for alternate name of hwy_data directory #595

jteresco opened this issue Jul 20, 2023 · 21 comments
Labels
railways TM expansion into railways

Comments

@jteresco
Copy link
Contributor

jteresco commented Jul 20, 2023

As part of an attempt to get a preliminary railway version of TM up and running, I noticed that the directory name hwy_data is hard-coded into the site update program. This could become a command-line parameter, or possibly we could make hwy_data the default name when the repository is called HighwayData and rail_data when the repository is called RailwayData.

@jteresco jteresco added the railways TM expansion into railways label Jul 20, 2023
@jteresco
Copy link
Contributor Author

An alternative is to rename it as plain data in both repositories, with the idea that it's obviously "highway data" or "rail data" based on the name of the repository. This would eliminate any need for future names like bike_data or ski_data or trail_data if TM continues to expand into other types of travel.

@yakra
Copy link
Contributor

yakra commented Jul 22, 2023

Plain data sounds reasonable. Keep it simple.


Similarly, maybe rename some classes, like
HighwaySegment -> Segment
HighwaySystem -> RouteSystem
etc.

@jteresco
Copy link
Contributor Author

jteresco commented Jul 29, 2023

I think I'd like the site update to look for data by default. I don't think it's worth renaming it in the HighwayData repository. Git should be able to maintain history if we do, but I worry about getting everyone who has a clone up to date, especially when they have partially completed work not yet committed to the origin repository. We should able to add a symbolic linkdata -> hwy_data. The other repositories new data repositories can easily enough just have them renamed as vanilla data. @yakra would you like me to make that change in site update code, or would you prefer to make it?

@jteresco
Copy link
Contributor Author

I'm going ahead and making this change since I have a good chance to do it tonight. Just in C++.

@yakra
Copy link
Contributor

yakra commented Jul 30, 2023

OK. Doesn't look like this would cause merge conflicts with anything I have in the pipeline.
Looks like 54fecf1 gets the 3 crucial files here. I'll take a closer look in a bit.

The crawl_hwy_data function still works of course, but may not have the most appropriate name anymore.
crawl_repo_data?
crawl_route_data?
crawl_data?

@jteresco
Copy link
Contributor Author

I was thinking about that as well. I'm not sure what term works best..

@jteresco
Copy link
Contributor Author

@yakra
Copy link
Contributor

yakra commented Jul 30, 2023

May be worthwhile to add an option to specify the data subdirectory, for compatibility with HighwayData commits before fe7ce91.


@jteresco wrote:

Just in C++.

Should this be taken as a semi-official "beginning of the end" for siteupdate.py?
It'll be compatible with existing HighwayData, but not new non-highway TM flavors going forward.

@yakra
Copy link
Contributor

yakra commented Aug 1, 2023

grep -ni HIGHWAYDATAPATH `find . -name \*.h` `find . -name \*.cpp`

This will produce merge conflicts.
Less important & can wait a bit, right?

@jteresco
Copy link
Contributor Author

jteresco commented Aug 1, 2023

Definitely can wait.

@yakra
Copy link
Contributor

yakra commented Aug 1, 2023

Simply DATAPATH, maybe?

@jteresco
Copy link
Contributor Author

jteresco commented Aug 1, 2023

Probably just DATAPATH is good. No rush at all on it.

@jteresco
Copy link
Contributor Author

jteresco commented Aug 1, 2023

Should this be taken as a semi-official "beginning of the end" for siteupdate.py? It'll be compatible with existing HighwayData, but not new non-highway TM flavors going forward.

Yes, I am comfortable with only C++ being updated from now on. I doubt we'd have need to go back to Python for anything production going forward. The only possible advantages I see in maintaining Python are that 1) it's an independent program that we can use to see if the C++ code is producing the same results, and 2) I know the code much better than I know the C++ code.

@yakra
Copy link
Contributor

yakra commented Aug 10, 2023

ToDo list

Gathering these items up in one post.
I'd like to hold off on making these changes, as right now there'd still be merge conflicts aplenty.

  • Rename HighwaySegment class (potentially to Segment?)
  • Rename HighwaySystem class (potentially to RouteSystem?)
  • Rename crawl_hwy_data function (crawl_route_data? crawl_data?)
  • Add option to specify the data directory. Fits in with the title of the issue. ;)
  • highwaydatapath -> datapath
  • Writing highway data stats log file (highwaydatastats.log).
  • Anything else left over
    egrep -ni 'highway|hwy' `find . -name \*.h` `find . -name \*.cpp`

@jteresco
Copy link
Contributor Author

Thanks for putting that ToDo list together. It looks good, but certainly is not a high priority.

@rickmastfan67
Copy link

Thanks for putting that ToDo list together. It looks good, but certainly is not a high priority.

Don't mind @yakra, he's just making busy work to avoid dealing with the 5 new threads a day for his regions in the forums. lol. :p

@michihdeu
Copy link
Contributor

Thanks for putting that ToDo list together. It looks good, but certainly is not a high priority.

I think that there are more (still virtual) ToDo lists in regards to the new categories, e.g. get started user manual, contributor manual, credits and sources, datacheck, NMP, wpt editor layers,....

route finder looks good (but needs more consistency in regards to route names). It might generally replace the RB...

@yakra
Copy link
Contributor

yakra commented Aug 13, 2023

LOL oy gevalt.
"I'll just bang out the TMArray conversion and then get back to the forum", I said.
"Easy as pie; it'll just take a couple hours", I said.
And now 4 weeks later, it's lead down a number of rabbit holes of tweaks and optimizations and How To Do It Right, and I'm just after rebasing 47 commits down to 22, and... cough.
I've got to force myself to set the small things aside, and am always opening new issues in yakra/DataProcessing to look into at a more appropriate time later, so I don't forget.


The ToDo list is mostly just for my benefit, really.
The only item on it visible to contributors or the general public will be Writing highway data stats log file (highwaydatastats.log).; it could be annoying seeing that in siteupdate.log for a non-highway TM flavor. :)
Other than that, everything else is just keeping appropriate/accurate/relevant names for things behind the scenes. Useful for me & Jim, and any other developer who may come along and look at the siteupdate codebase in the future.
Quick & easy changes to make, once the time comes to do so.

The items michih mentions are important of course, though probably better suited to Web/ issues.
My focus here is just on keeping sensible names on the DataProcessing side, admittedly a bit tangential to the OP.

@jteresco
Copy link
Contributor Author

The items michih mentions are important of course, though probably better suited to Web/ issues. My focus here is just on keeping sensible names on the DataProcessing side, admittedly a bit tangential to the OP.

Yes, @michihdeu and others are welcome and encouraged to open over in the Web repository for things that would improve the support there for the new railways data.

@yakra
Copy link
Contributor

yakra commented Aug 22, 2023

Finally finalizing the big TMArray conversion. I expect to have the pull request ready in the next few days. Hopefully.
It makes sense to do a few of the ToDo list items at the same time, while saving some others for the future for sanity's sake.

  • highwaydatapath -> datapath

    Already done in my local copy. What's not changed yet is the description in siteupdate -h / siteupdate --help:
    path to the root of the highway data directory structure.

    • Shall we just say "route data" in places where we've till now said "highway data"?
    • Is additional clarification that it's the root of the repo itself rather than the data/ dir worthwhile?
      E.G., path to the root directory of the route data repository.
      Or, we could shorten it up, path to the root of the route data repository. or even path to the route data repository.
      Thoughts?
  • Rename crawl_hwy_data function

    I think I'll go with crawl_rte_data. The name's the same length, and just plain "data" feels a bit too generic, maybe ambiguous or confusing.

  • Writing highway data stats log file (highwaydatastats.log).

    I almost wanna go with just a short datastats.log, but again that feels to generic & nondescript.
    Is routedatastats.log OK?

@jteresco
Copy link
Contributor Author

I'm good with whatever you think on these kinds of things. I wouldn't worry too much about picking perfect names.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
railways TM expansion into railways
Projects
None yet
Development

No branches or pull requests

4 participants