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

Remove stimfitio from pyproject #1602

Merged
merged 1 commit into from
Dec 3, 2024

Conversation

h-mayorquin
Copy link
Contributor

As discussed with @samuelgarcia and @zm711

this has not been working for a while.

Trying to install it generate this error.

pip install neo[stimfitio]==0.13.4
Collecting neo==0.13.4 (from neo[stimfitio]==0.13.4)
  Using cached neo-0.13.4-py3-none-any.whl.metadata (8.9 kB)
Requirement already satisfied: packaging in ./miniconda3/envs/test_env/lib/python3.11/site-packages (from neo==0.13.4->neo[stimfitio]==0.13.4) (24.1)
Requirement already satisfied: numpy<2.0.0,>=1.22.4 in ./miniconda3/envs/test_env/lib/python3.11/site-packages (from neo==0.13.4->neo[stimfitio]==0.13.4) (1.26.4)
Collecting quantities>=0.16.1 (from neo==0.13.4->neo[stimfitio]==0.13.4)
  Using cached quantities-0.16.1-py3-none-any.whl.metadata (8.4 kB)
INFO: pip is looking at multiple versions of neo[stimfitio] to determine which version is compatible with other requirements. This could take a while.
ERROR: Could not find a version that satisfies the requirement stfio; extra == "stimfitio" (from neo[stimfitio]) (from versions: none)
ERROR: No matching distribution found for stfio; extra == "stimfitio"

And if you remove the version it goes back to a version of python-neo that did not have that extra and you end up with version 0.5.

This PR removes this.

@h-mayorquin
Copy link
Contributor Author

Mysterious failure of one test, @zm711 . Any hints on this?

@apdavison apdavison changed the title Remove tiffio from pyproject Remove stimfitio from pyproject Nov 29, 2024
@apdavison
Copy link
Member

this makes sense since stfio cannot be pip-installed, as per the comment below the "all" target.

@h-mayorquin
Copy link
Contributor Author

h-mayorquin commented Nov 29, 2024

Would you be OK with removing it, @apdavison ? Right now this is working as text documentation which is a plus but it also breaks some tooling. I was mentioning Sam that uv tree (a nice and efficient tool to build the dependency tree) is unable to work because stfio is not on an index (e.g. pipy) as far as I know. In addition, it also has the strange behavior described above with normal pip where you end up installing a really old version of neo.

How do you think about it?

[edit, I misquoted Andrew]

Copy link
Contributor

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

Since I don't know stimfitio at all, I feel like we should make sure there is an appropriate error in the io that let's people know what's going on. Have you checked this @h-mayorquin ?

Also I reran tests to see if it's stochastic failures (which we will need to fix--but would be a separate issue).
--It is a random failure. We can track this if it comes up again. This was in filtering code (another section that I haven't explored very deeply).

@h-mayorquin
Copy link
Contributor Author

ll, I feel like we should make sure there is an appropriate error in the io that let's people know what's going on.

What sort of error do you have in mind?

@zm711
Copy link
Contributor

zm711 commented Dec 2, 2024

ll, I feel like we should make sure there is an appropriate error in the io that let's people know what's going on.

What sort of error do you have in mind?

That's just it, I'm not sure. I honestly don't know what is necessary to run this IO if it is not a pypi package. So in this case I would defer to @apdavison or @samuelgarcia if they know what an appropriate warning would be. Or if they are fine with not having a warning.

@apdavison
Copy link
Member

It is packaged for Debian, MacPorts, Homebrew, Windows, see:

https://github.com/neurodroid/stimfit?tab=readme-ov-file#installation

@h-mayorquin
Copy link
Contributor Author

I think that right now if someone was to use StimfitIO and they don't have the library you will get a classical import error.

# We need this module, so try importing now so that it fails on
# instantiation rather than read_block
import stfio # noqa

This seems good enough for me. You could use importlib or the try-except pattern with imports to get a more specific import error but I kind of feel that's an overkill.

Copy link
Contributor

@zm711 zm711 left a comment

Choose a reason for hiding this comment

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

Works for me then :)

What is this even doing in the pyproject then...?

@zm711 zm711 added this to the 0.14.0 milestone Dec 2, 2024
@zm711 zm711 merged commit 2576037 into NeuralEnsemble:master Dec 3, 2024
48 checks passed
@h-mayorquin h-mayorquin deleted the fix_pyproject_toml branch December 3, 2024 14:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants