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

Implementation of Times projection #400

Merged
merged 1 commit into from
Aug 22, 2016
Merged

Conversation

kbevers
Copy link
Member

@kbevers kbevers commented Jul 27, 2016

Michał Siwicki requested on the mailing list that the Times projection was added to proj.4. This PR takes care of that. The implementation is based on Snyder's "Flattening the Earth".

Here's how it looks:
out


all: proj.lib $(PROJ_EXE) $(CS2CS_EXE) $(GEOD_EXE) $(NAD2BIN_EXE)

proj.lib: $(LIBOBJ)
if exist proj.lib del proj.lib
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think you really wanted to remove all these tabs.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh, it was definitely intentional - tabs has no place in modern computer code. I'm slowly getting rid of them whenever I come across a file with tabs. There is quite a lot of files that mixes tabs and spaces in indentations which makes the code hard to read for most people.

But thanks for pointing it out. Actually you made me think about the consequences, so I've made a recommitted the file without removing tabs since a few of the other open PR's will be harder to merge with the tabs->spaces conversion.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

tabs has no place in modern computer code

Please be careful about changing the "shape" of lots of existing code for whitespace purity. I don't care for tabs so much either, but if the code is consistently one or the other, IMO it should be left alone. If it isn't consistent, fix it 😄

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree, which is also why I did not do anything about Makefile.am. It has consistent use of tabs for indentation. Most files in proj are a mixture, and it always looks horrible. I try as best as possible to keep the intended indentation levels but that can be difficult as well; often there's several different styles in the same file.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Makefiles, it's not so much consistency that's the issue but the
requirement that the commands for a rule be indented with a TAB
character.

Also if tabbed code is looking horrible, it's possible that your editor
needs to be adjusted for a different width tab. On Unix machines (and
earlier), a TAB was 8 spaces by default. MS Visual Studio seems to use
a TAB of 4 spaces.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With Makefiles, it's not so much consistency that's the issue but the
requirement that the commands for a rule be indented with a TAB
character.

Ah, I was not aware of that! I've never made one of those from scratch, so never had a reason dig into all the details.

I am well aware that you can have different tab widths. That's why tabs are mostly bad in my opinion. Code looks different on different setups, which is fine if tabs are used consistently. That's rarely the case for the code in proj and leads to people indenting code with spaces to whatever tab width is in their particular editor. In my editor I have tabs showing up as "^I" to make them super obvious (I like to know what is going on, and have been fooled by stray tabs before...). This also makes mixed indentations look just a bit more horrible and scream at you to be fixed :)

@kbevers kbevers force-pushed the times-projection branch 2 times, most recently from 3c4ce66 to 98ffd27 Compare July 28, 2016 10:57
@kbevers kbevers added this to the 4.10.0 milestone Jul 28, 2016
@hobu
Copy link
Contributor

hobu commented Jul 29, 2016

@kbevers how are you making this picture, and is it something we could automate for the generation of figures for the website materials?

@kbevers
Copy link
Member Author

kbevers commented Jul 29, 2016

@hobu It's a python script I've been working on. My motivation was precisely what you are suggesting: Automating figure generation for documentation.

I've made a copy here: https://gist.github.com/kbevers/9fa5c4708ab37c465313f7bc17ced252

It's still work in progress. For now it only works for some of the simpler projections like the Times projection showed above. Healpix for instance does not produce a pretty picture at all. I'll probably follow a different path later and use some of the in-build stuff in matplotlib. I believe it is possible to link it with proj and the use the cartographic features. Should be a better way!

@julien2512
Copy link
Contributor

@kbevers I am not sure if
http://spatialreference.org/ref/sr-org/7670/
is equivalent with yours ? It seems to use a rimes projection already.

@julien2512
Copy link
Contributor

"times" projection of course. My smartphone can't stand it !

@kbevers
Copy link
Member Author

kbevers commented Jul 31, 2016

@julien2512 It definitely seems to be referring to the same projection. It's not been implemented in proj before though. Also, if you follow the link to the proj-definition you get an empty page which I guess is spatialreference.org's way of saying that it can't be done...

@julien2512
Copy link
Contributor

You are probably right ! I guess they will change it after the master commit (or I will tell them).

@rouault
Copy link
Member

rouault commented Aug 9, 2016

Note: spatialreference.org is no longer maintained and hasn't been updated in years. It reflects the states of GDAL, proj.4 from several years ago.

@julien2512
Copy link
Contributor

julien2512 commented Aug 9, 2016

@rouault So to make me to maintain it. Many users still using it ! I will check it up later. Tks.

@hobu hobu modified the milestones: 4.9.3, 4.10.0 Aug 17, 2016
@hobu hobu merged commit 307914d into OSGeo:master Aug 22, 2016
@kbevers kbevers deleted the times-projection branch October 14, 2016 12:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants