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 no-op operation. It does nothing. #1391

Merged
merged 2 commits into from Apr 2, 2019
Merged

Add no-op operation. It does nothing. #1391

merged 2 commits into from Apr 2, 2019

Conversation

@kbevers
Copy link
Member

@kbevers kbevers commented Mar 28, 2019

Closes #1185

@kbevers kbevers added this to the 6.1.0 milestone Mar 28, 2019
@rouault
Copy link
Member

@rouault rouault commented Mar 28, 2019

noop.cpp lacking in the commit

The following line could be upgraded to use noop

../src/iso19111/c_api.cpp:                ctx, projString.empty() ? "+proj=affine" : projString.c_str());

And, a bit harder, projinfo -s EPSG:4326 -t EPSG:4326 -o PROJ currently returns an empty string. I think that the fix would be to return "+proj=noop" at the end of PROJStringFormatter::toString() in io.cpp if the string is empty. And in that case the above test of the empty string in c_api.cpp could be just removed.

Loading

@kbevers
Copy link
Member Author

@kbevers kbevers commented Mar 28, 2019

noop.cpp lacking in the commit

🤦

The following line could be upgraded to use noop

I was looking for that but couldn't locate it in my first attempt. I had hunch you would remind me where it was :-) I will see if I can figure out how to do it the hard way. If not, I'm taking the easy path...

Loading

@kbevers
Copy link
Member Author

@kbevers kbevers commented Mar 28, 2019

I've added a very naive adoption of noop in iso19111 code. I saw some test failures locally so this probably won't work here either. It's a starting point for further discussion :)

Loading

@rouault
Copy link
Member

@rouault rouault commented Mar 28, 2019

I saw some test failures locally so this probably won't work here either

from Travis output, at least the first failures are no real issues. testprojinfo_out just needs to be updated with the new output

Loading

@kbevers
Copy link
Member Author

@kbevers kbevers commented Mar 29, 2019

from Travis output, at least the first failures are no real issues. testprojinfo_out just needs to be updated with the new output

Yeah, it does seem like that. I had a problem locally that I couldn't immediately determine if was from this change or something else. I've updated the test output for testprojinfo.

Loading

@kbevers kbevers merged commit b8a6252 into OSGeo:master Apr 2, 2019
3 checks passed
Loading
@kbevers kbevers removed this from the 6.2.0 milestone Apr 2, 2019
@kbevers kbevers added this to the 6.1.0 milestone Apr 2, 2019
@kbevers kbevers deleted the noop branch Apr 18, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants