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

Merge RFC4 #1865

Merged
merged 80 commits into from
Jan 22, 2020
Merged

Merge RFC4 #1865

merged 80 commits into from
Jan 22, 2020

Conversation

rouault
Copy link
Member

@rouault rouault commented Jan 22, 2020

This is the PR merging RFC4 to master.

@kbevers Do you prefer this squashed or merged ?

…hgridshift/vgridshift/deformation structures
…stward correction' in the CTable2/NTv1/NTv2 backends
…grid. Used for French NTF to RGF93 transformation using gr3df97a.tif grid
…n switching between (sub)grids (fixes #1663)

Given in.txt with
53.999759140 5.144478208 252.6995

Before the fix,
cct  -t 0 -d 4 +proj=pipeline +step +proj=axisswap +order=2,1,3,4 +step +proj=hgridshift +inv +grids=rdtrans2018.gsb +step +proj=vgridshift +grids=naptrans2018.gtx +step +proj=sterea +lat_0=52.156160556 +lon_0=5.387638889 +k=0.9999079 +x_0=155000 +y_0=463000 +ellps=bessel in.txt

returned:
  139079.8814    668306.0302      212.1724        0.0000

It now returns:
  139079.8850    668306.0458      212.1724        0.0000

which meets with the 1mm accuracy the expected result of test point
```
30010049	53.999759140	5.144478208	252.6995	139079.8850	668306.0460	212.1723
```
[RFC4_dev] Add networking capabilities
[RFC4_dev] deformation: add support for +grids= for GeoTIFF grids
[RFC4_dev] Add a SQLite3 local cache of downloaded chunks
- Add a usage/network.rst document for all network related information (fixes #1852)
- reference it from cs2cs and cct docs
- Create a specification/ section
- Move usage/projjson.json to specification/
- Move the description of the GeoTIFF format from RFC-4 text to a
  specifications/geodetictiffgrids.rst (refs #1850)
[RFC4_dev] Address code review comments
[RFC4_dev] Use Win32 Unicode APIs and expect all strings to be UTF-8 (fixes #1765)
[RFC4_dev] Merge master back to rfc4 latest branch
@kbevers
Copy link
Member

kbevers commented Jan 22, 2020

This is the PR merging RFC4 to master.

Hooray! Well done, Even. You have pulled of an impressive piece of work.

@kbevers Do you prefer this squashed or merged ?

Ideally I would like this to be divided into commits corresponding roughly to the pull requests you've done. That may be a rather complicated git rebase effort. If it is too difficult to do I think it is best to just merge as is. One massive squashed commit would make it difficult to find the root cause of a bug using git bisect. There will be other downsides as well.

@rouault
Copy link
Member Author

rouault commented Jan 22, 2020

If it is too difficult to do

I think it indeed goes beyond my git capabilities. Especially since there was a merge from master to rfc4_dev at some point, so I think the rebasing model would completely break. I'll go to the regular merge then.

@kbevers
Copy link
Member

kbevers commented Jan 22, 2020

I just tried to squash some related commits in an interactive rebase. It is doable but not without effort. Some conflicts will arise. After a more careful look at the commits I think most of them are fine as they are. Some of the commits adressing reviews could be squashed into their original commits to provide a better commit history, I would think that is fairly easy to do.

@rouault
Copy link
Member Author

rouault commented Jan 22, 2020

I would think that is fairly easy to do.

Well, I just tried and miserably failed. I've checkout rfc4_dev and do a git rebase -i master, then I tried to move commit "xyzgridshift.rst: fix typo" just after "Add a +proj=xyzgridshift method to perform geocentric translation" to squash them together, but it fails when applying the very first commit of rfc4_dev.

@kbevers
Copy link
Member

kbevers commented Jan 22, 2020

Well, I just tried and miserably failed

okay, bummer. I don't know if it matters but I tried with git rebase -i 41ff947~ on rfc4_dev and squashed all the commits starting with "Network:". It resulted in one merge conflict that at first glance didn't look to difficult to fix. It would probably still be difficult to merge into master without to many conflicts.

@rouault
Copy link
Member Author

rouault commented Jan 22, 2020

git rebase -i 41ff947~

OK, that's a bit better indeed, but when just applying that and validating, I get conflicts exactly at the points where I got conflict when merging master back to rfc4_dev a few days ago. I'm afraid solving them could result in a history where some points wouldn't be buildable. I think mixiing git merge + git rebase is a big no no in git practices.

I've also considered to try to redo a clean history by manual merging, but the effort+risk of forgetting something is discouraging

I think the squash everything button of github might be the best option.

@kbevers
Copy link
Member

kbevers commented Jan 22, 2020

I'm afraid solving them could result in a history where some points wouldn't be buildable

Yes, each commit should preferable the repo in a buildable state.

I think the squash everything button of github might be the best option.

All things considered I think you are right. It would be great if you could update the commit message, giving a big picture overview of whats been done instead of just the list of commit headers GitHub usually puts in as the commit message when squashing

@rouault
Copy link
Member Author

rouault commented Jan 22, 2020

It would be great if you could update the commit message, giving a big picture overview of whats been done instead of just the list of commit headers GitHub usually puts in as the commit message when squashing

sure. What do think of the following ?

This commit is the result of the squashing of rfc4_dev branch in a single
commit. It implements mostly RFC 4 related work.

* Grid handling:
  - remove obsolete and presumably unfinished implementation of grid catalog functionality
  - all grid functionality is in grids.cpp/.hpp
  - vertical and horizontal grid shift: rework to no longer load whole grid into memory
  - remove hgrids and vgrids member from PJ structure, and store them in hgridshift/vgridshift/deformation structures
  - build systems: add optional libtiff dependency. Must be explicitly disabled if not desired
  - add support for horizontal and vertical grids in GeoTIFF, if libtiff is available
  - add GenericShiftGridSet and GenericShiftGrid classes, relying on TIFF grids, that can be used for generic purpose grid-based adjustment
  - add a +proj=xyzgridshift method to perform geocentric translation by grid. Used for French NTF to RGF93 transformation using gr3df97a.tif grid
  - deformation: add support for +grids= for GeoTIFF grids
  - horizontal grid shift: fix failures on points slightly outside a subgrid (fixes #209)

* File management:
  - add a filemanager.cpp/.hpp to deal with file related work
  - test for legacy proj_api.h fileapi
  - proj.h: add proj_context_set_fileapi() and proj_context_set_sqlite3_vfs_name() (fixes #866)
  - add capability to read resource files from the user writable directory

* Network access:
  - build systems: add optional curl dependency
  - add a curl-based default implementation for network related functionality
  - proj.h: add C API to control network functionality, and optionaly provide network callbacks
  - add data/proj.ini with default settings
  - add a SQLite3 local cache of downloaded chunks
  - add proj_is_download_needed() and proj_download_file()

* Use Win32 Unicode APIs and expect all strings to be UTF-8 (fixes #1765)
  For backward compatibility, if PROJ_LIB content is found to be not UTF-8 or
  pointing to a non existing directory, then an attempt at interpretating it
  in the ANSI page encoding is done.
  proj_context_set_search_paths() now assumes strings to be in UTF-8, and
  functions returning paths will also return values in UTF-8.

@kbevers
Copy link
Member

kbevers commented Jan 22, 2020

I think that is great @rouault. Feel free to merge :)

@rouault rouault merged commit db31b6d into master Jan 22, 2020
@rouault
Copy link
Member Author

rouault commented Jan 22, 2020

Done !!!! :-)

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

Successfully merging this pull request may close these issues.

2 participants