Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Renaming proj4 to proj.4 #116

Merged
merged 1 commit into from
Nov 3, 2015
Merged

Renaming proj4 to proj.4 #116

merged 1 commit into from
Nov 3, 2015

Conversation

ocefpaf
Copy link
Member

@ocefpaf ocefpaf commented Oct 10, 2015

Just experimenting. This PR:

  • Removes proj4 4.9.1
  • Change cartopy to use the default channel version of Proj4 (proj.4 4.9.1)

Note that the default channel does not provide version 4.9.2 of Proj4 yet.
Anyways, I am not sure if that version is OK for cartopy. See ioos/conda-recipes#500.

number: 0 # [not linux]
number: 1 # [linux]
number: 1 # [not linux]
number: 2 # [linux]
Copy link
Member

Choose a reason for hiding this comment

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

Now that they're both bumped, can we not just make them the same?

Copy link
Member Author

Choose a reason for hiding this comment

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

If we bump both to release 2 then the channel would not have release 1 for OSX and Windows. It is a minor inconsistency (and I do that in the IOOS channel all the time), but that does not seems to be the way @pelson do things here.

Copy link
Member

Choose a reason for hiding this comment

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

I'd be happy with that.

Copy link
Member Author

Choose a reason for hiding this comment

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

Done!

@ocefpaf ocefpaf force-pushed the rename_proj4 branch 2 times, most recently from 9eb2a5c to 5424b2a Compare October 11, 2015 17:49
@ocefpaf ocefpaf changed the title Using default proj4 Renaming proj4 to proj.4 Oct 11, 2015
@ocefpaf ocefpaf force-pushed the rename_proj4 branch 2 times, most recently from ed67c60 to 04b12ef Compare October 11, 2015 22:10
@ocefpaf
Copy link
Member Author

ocefpaf commented Oct 11, 2015

The AppVeyor failure is fixed in #114. And I am almost certain that the circleci failure is a conda bug that is already fixed in the latest version.

@pelson do you want to add a conda update --yes --all to the run_docker_build.sh file or do you want to update the docker image?

@pelson
Copy link
Member

pelson commented Oct 12, 2015

Notice the numpy build string isn't in the cartopy distribution:

    cartopy-0.13.0-py27_1 (will be built: True)
    cartopy-0.13.0-py27_1 (will be built: True)
    cartopy-0.13.0-py34_1 (will be built: True)

Would you mind adding the necessary ?? to the recipe?

@ocefpaf
Copy link
Member Author

ocefpaf commented Oct 12, 2015

Notice the numpy build string isn't in the cartopy distribution:

I thought that numpy * would take care of that 😒

Would you mind adding the necessary ?? to the recipe?

I will make a few tests and see if I can fix that.


if [[ $(uname) == Linux ]]; then
make check
fi
Copy link
Member Author

Choose a reason for hiding this comment

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

NB make check fails in OSX! We did not run check before.

Copy link
Member

Choose a reason for hiding this comment

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

For 4.9.2 as well? What is the error? We should push that up to proj.4.

Copy link
Member Author

Choose a reason for hiding this comment

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

For 4.9.2 as well?

Don't remember, but I had other issues building cartopy on Windows against Proj.4 4.9.2.

What is the error?

https://travis-ci.org/ioos/conda-recipes/builds/84817022

We should push that up to proj.4.

Not sure what you mean.

Copy link
Member

Choose a reason for hiding this comment

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

That build is failing with cdo, not proj.4.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep. I mixed the logs here. Let me see if I can find it. (Or better yet, remove the conditional and see if it fails again.)

Copy link
Member Author

Choose a reason for hiding this comment

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

@QuLogic

Here is Proj 4.9.2 failure: https://travis-ci.org/ioos/conda-recipes/builds/85402318

And as soon as travis finishes we will know what happens here with proj 4.9.1

Copy link
Member Author

Choose a reason for hiding this comment

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

Same error: https://travis-ci.org/SciTools/conda-recipes-scitools/builds/85403903

I am removing that make check again.

Copy link
Member

Choose a reason for hiding this comment

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

Can you try adding this patch?

diff --git a/src/rtodms.c b/src/rtodms.c
index 591874a..96231ff 100644
--- a/src/rtodms.c
+++ b/src/rtodms.c
@@ -63,7 +63,7 @@ rtodms(char *s, double r, int pos, int neg) {
                if (*p != '.')
                        ++p;
                if (++q != p)
-                       (void)strcpy(p, q);
+                       (void)memmove(p, q, (sign ? 3 : 2));
        } else if (min)
                (void)sprintf(ss,"%dd%d'%c",deg,min,sign);
        else

Copy link
Member

Choose a reason for hiding this comment

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

Looks like it worked, so OSGeo/PROJ#317.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yep that did the trick. Thanks @QuLogic.

@ocefpaf
Copy link
Member Author

ocefpaf commented Oct 12, 2015

I am trying a different approach here to get more control over the build string. I just noticed that it is gone from a few other packages too. I am not sure if the disappearance of the build string was something I changed or a conda upgrade.

Right now change fad5239 produces cartopy-0.13.0-npNonepy27_2.

None would be replaced by the proper numpy build string if we have either the CONDA_NPY=$VER env variable or if we call the build with conda build --numpy=$VER ....

@pelson does Obvious-CI creates the $VER variable for numpy? Is this an approach you want to pursue?

If that works and you are OK with it I can propagate that change to the other packages.

@ocefpaf
Copy link
Member Author

ocefpaf commented Oct 12, 2015

Actually that produced all these:

  • cartopy-0.13.0-npNonepy27_2
  • cartopy-0.13.0-np18py27_2
  • cartopy-0.13.0-np19py27_2
  • cartopy-0.13.0-np110py27_2

I guess we just need to find a way to get rid of cartopy-0.13.0-npNonepy27_2.

@ocefpaf ocefpaf force-pushed the rename_proj4 branch 4 times, most recently from 6da1c21 to 59f86ca Compare October 14, 2015 21:49
@pelson
Copy link
Member

pelson commented Oct 15, 2015

@pelson does Obvious-CI creates the $VER variable for numpy? Is this an approach you want to pursue?

Yep, I'll take a look at upgrading obvious-ci for the latest conda-build. I will also update to use skip properly (see https://github.com/pelson/Obvious-CI/issues/32).

@ocefpaf ocefpaf force-pushed the rename_proj4 branch 3 times, most recently from 9f485b3 to 8823179 Compare October 30, 2015 14:59
@pelson
Copy link
Member

pelson commented Oct 31, 2015

@ocefpaf - would you mind re-basing this?

@ocefpaf
Copy link
Member Author

ocefpaf commented Oct 31, 2015

Done!

pelson added a commit that referenced this pull request Nov 3, 2015
@pelson pelson merged commit 5cc85ed into SciTools:master Nov 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants