mdhaber / marray

Masked versions of array API compatible arrays
MIT License
4 stars 1 forks source link

TST: filter NumPy 'invalid' warnings; comment out failing tests #10

Closed mdhaber closed 1 day ago

mdhaber commented 1 day ago

Thought I'd give CI a test run.

mdhaber commented 1 day ago

@keewis As you can probably tell, I don't know much about project/CI configuration. Do you know what the error message on the Windows job means?

ERROR: Package 'marray' requires a different Python: 3.9.13 not in '>=3.10'

I'm surprised to see this for Windows but not other platforms.

keewis commented 1 day ago

Do you know what the error message on the Windows job means?

the python version currently doesn't do anything because I forgot to add a setup-python step: the CI is more or less copied from xarray, which uses setup-micromamba to setup the environment, and because I thought we wouldn't need a conda env here I removed the setup-micromamba step but didn't add a replacement (and apparently the windows image by default uses python 3.9).

This passed through because Github doesn't want to run CI created by outside contributors, so I couldn't use passing / failing checks as a clue to start debugging.

I'll send in another PR to fix that.

keewis commented 1 day ago

separately, though, what do you think about venv vs conda envs?

My take is: marray has minimal dependencies (numpy, which may be replaced by array-api-compat?) so using a venv might be fine for now, and I'd say the point when we should start using conda envs instead is when we need more binary packages.

mdhaber commented 1 day ago

This passed through because Github doesn't want to run CI created by outside contributors,

I'll add you. I might directly add to the test suite like I was doing before and make little fixes to the existing implementations as needed, but otherwise, let's continue to work through PRs.

separately, though, what do you think about venv vs conda envs?

For CI? I don't have an informed opinion. Either is fine with me.

I don't think it will have any required dependencies - just optional dependencies like pytest, array-api-compat, and any other array libraries for testing. (It came to mind that at some point we might want to make adaptations for libraries that don't support mutating arrays, like jax.numpy, and then add them to tests. I am going to ignore this initially, though.)

mdhaber commented 1 day ago

OK with you to ignore the NumPy invalid warnings for now? If so, please feel free to hit the big green button (squash please).

keewis commented 1 day ago

I don't think it will have any required dependencies - just optional dependencies like pytest, array-api-compat, and any other array libraries for testing.

I guess I should have said "test dependencies", since that's what is relevant for the environments. But otherwise we can switch to conda whenever it becomes an issue (like if jax installed from PyPI does not work properly)

squash please

:+1: You can actually forbid other types of merges in the repository settings, so I won't even have the option to not squash merge

mdhaber commented 1 day ago

You can actually forbid other types of merges in the repository settings

I like it to be an option. If the history is clean and meaningful, regular merge is OK. Here, I had a one-character fixup, so squash would be better.

keewis commented 1 day ago

okay, looks like the remaining concerns are resolved. Let's merge, then!