Skip to content

Conversation

@samuelgarcia
Copy link
Contributor

@yger @alejoe91
Could you test this ?

@pep8speaks
Copy link

pep8speaks commented Apr 30, 2021

Hello @samuelgarcia! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

Line 78:100: E501 line too long (111 > 99 characters)
Line 194:79: W291 trailing whitespace
Line 213:100: E501 line too long (113 > 99 characters)
Line 216:100: E501 line too long (116 > 99 characters)
Line 219:100: E501 line too long (113 > 99 characters)

Comment last updated at 2021-06-18 10:19:38 UTC

@samuelgarcia
Copy link
Contributor Author

@yger @alejoe91 : can you make feedback on this soon so we could merge this ?

@samuelgarcia
Copy link
Contributor Author

As soon as files are merge in gin. This is ready to review.

@alejoe91
Copy link
Contributor

@samuelgarcia maybe a possible solution is to add the libcompression.so in the maxwell folder and add the env variable before testing?

@samuelgarcia
Copy link
Contributor Author

I prefer not include the libcompression.so in the code. We need permission for that.
Maybe we could write a libcompression.so retriever but I don't have time for that at the moment.
I think the error message is enough and is already a big improvement for user compare to previous extractor no ?

@alejoe91
Copy link
Contributor

Yes. I asked maxwell to make it downloadable via wget

@samuelgarcia
Copy link
Contributor Author

OK we can try a:

  • wget in the yml of github actions
  • wget in python code (more work)

@samuelgarcia
Copy link
Contributor Author

@JuliaSprenger : can you have a look to this I patch the download_dataset() and add the update stage.
So distant changes will be updated locally now.
When this will be merge other PR will be OK.

@samuelgarcia
Copy link
Contributor Author

@alejoe91 : I add a util function auto download this plugin.
test are still failing. I prefer to disable for now.
We will see this later.

@samuelgarcia
Copy link
Contributor Author

Ready to review.

@JuliaSprenger JuliaSprenger self-requested a review May 21, 2021 09:29
@samuelgarcia
Copy link
Contributor Author

@apdavison : If this pass tests, could we merge this soon ?
This PR will unblock other PR and will fix the file caching.
Now any PR take more than 1 hour.

…into maxwell_rawio_implementation

# Conflicts:
#	neo/utils/datasets.py
@samuelgarcia
Copy link
Contributor Author

Ready to review (and merge maybe)

@JuliaSprenger JuliaSprenger merged commit 6c84ac3 into NeuralEnsemble:master Jun 18, 2021
@samuelgarcia samuelgarcia deleted the maxwell_rawio_implementation branch March 9, 2022 11:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants