search

conda-forge / openquake.engine-feedstock

A conda-smithy repository for openquake.engine.
BSD 3-Clause "New" or "Revised" License
0 stars 9 forks source link

Remove test case with non-ASCII characters b/c it is 2020 and we still cannot have nice things #30

Closed ocefpaf closed 3 years ago

ocefpaf commented 3 years ago

Closes #27

conda-forge-linter commented 3 years ago

Hi! This is the friendly automated conda-forge-linting service.

I just wanted to let you know that I linted all conda-recipes in your PR (recipe) and found it was in an excellent condition.

ocefpaf commented 3 years ago

@emthompson-usgs I can reproduce the error you are experimenting. Working on a fix now...

emthompson-usgs commented 3 years ago

Since the error message was that the file is expected because it is in the manifest, but not cannot be found, I was wondering if you think it would make sense to try to remove the two lines in the manifest that include the testing data:

# QA tests data
recursive-include openquake/qa_tests_data *.* README
recursive-exclude openquake/qa_tests_data *.pyc
ocefpaf commented 3 years ago

Since the error message was that the file is expected because it is in the manifest, but not cannot be found

I'm not sure that is the failure here b/c it only happens on Windows, while we are removing that file from all builds.

I was wondering if you think it would make sense to try to remove the two lines in the manifest that include the testing data:

# QA tests data
recursive-include openquake/qa_tests_data *.* README
recursive-exclude openquake/qa_tests_data *.pyc

We can try but there are already a few workarounds in this packages. We are breaking the dependency pinning in the metadata, removing files in the MANIFEST, etc. It would be better to solve those issues upstream and as for a new release.

emthompson-usgs commented 3 years ago

Thanks for looking at this and trying to fix it @ocefpaf. I agree that the solution would be better handled upstream, but we aren't part of the OpenQuake development team and they don't use conda. Ideally, they would maintain the conda package. We only do it because we use conda to manage our dependencies, and make heavy us of OpenQuake. I'll close out these PRs and the associated issue and try to coordinate with the OpenQuake to find a solution.

ocefpaf commented 3 years ago

I'll get my hands on a Windows machine later today and I'll give it a try. So far no success in #30 but I still have a few tricks I want to try...

ocefpaf commented 3 years ago

@emthompson-usgs this should work on Windows. I just tested it locally but for reason I cannot make it work on Azure (more my lack of understanding of Windows than anything else.)

BTW, while trying to fix this upstream would be nice, something like splitting the test cases from the main package to make it easier to package and ship, this is really a bug in conda. It should handle those special characters on Windows.

github-actions[bot] commented 3 years ago

Hi! This is the friendly conda-forge automerge bot!

I considered the following status checks when analyzing this PR:

Thus the PR was passing and merged! Have a great day!

emthompson-usgs commented 3 years ago

Thanks so much for your efforts on this!

ocefpaf commented 3 years ago

Let me know if that works for you.

emthompson-usgs commented 3 years ago

@ocefpaf I've installed it on our Windows system and ran some tests. It is indeed working! This was a big step towards being able to support Windows systems for our group. If you know of any example conda packages that use cython and build on Windows that I can use as an example to follow, please let me know.

ocefpaf commented 3 years ago

If you know of any example conda packages that use cython and build on Windows that I can use as an example to follow, please let me know.

Are you adding a new package? Is it on PyPI? If so you can try grayskull to generate the recipe and it should do the right thing if the package has the proper metadata. I recommend to send a PR to staged-recipes and asking for help there for the details of the recipe.

emthompson-usgs commented 3 years ago

It isn't new or on PyPi. It is just a package that uses cython. It works fine in linux/OSX, and I am in the process of trying to get it to support Windows. Given my lack of familiarity with Windows I was thinking that having an example conda package that deals with the C compiler for Windows builds would be helpful.

ocefpaf commented 3 years ago

Let's follow up on that in the feedstock. It looks like there is no skip for Windows but a re-render may be missing.