Skip to content

Conversation

@tleeuwenburg
Copy link
Contributor

Weird file deletions have gone. Rebase done. Sorry for before :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

Why the camel case here only?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't know ... I think it's because of habit when using the unittest framework, I think it magically recognised that particular name, so it "has" to be camel case to be discovered appropriately. My knowledge could be stale, but that's why.

@fchollet
Copy link
Collaborator

fchollet commented Jul 3, 2015

Cool! So are some tests still not passing? Which?

@tleeuwenburg
Copy link
Contributor Author

All three tests in tests/auto/test_embeddings.py and tests/auto/test_regularizers.py are failing. Haven't investigated yet, but these exist on master and "someone" (possibly you) should be running py.test before committing to master.

There is another failing test in our work in test_constraints.py which seems to be caused by an actual problem in the method implementation which is using the symbol 'e' but it's not a variable and not imported from anywhere. I tried math.e in its' place, but it caused another issue. I think the method got refactored to match the new "instance" style for constrain but not checked in any way, and needs to be rewritted to used theano.exp() functions or something.

@fchollet
Copy link
Collaborator

fchollet commented Jul 3, 2015

I have fixed the constraints tests. The loss weighting test succeeds when run manually but fails on py.test; is there something about pytest I am not getting?

fchollet added a commit that referenced this pull request Jul 3, 2015
New pull request -- much cleaner
@fchollet fchollet merged commit 98f0550 into keras-team:master Jul 3, 2015
@tleeuwenburg
Copy link
Contributor Author

It's possible to get unexpected interactions with pytest and your main
python interpreter depending on both your classpath/pythonpath, and any
installed packages you might have. When I'm working on my fork, I have a
separate Python environment (I use anaconda for this) which doesn't have
keras installed, and manaully add my fork directory to the PYTHONPATH
environment variable.

That helps keep things straight between the current release, the master
source version and my forked version.

If you're using the 'system' py.test, you might (for example) be picking up
some packages you didn't expect but have installed into your OS. Using the
anaconda approach, the virtual environment has its own install of py.test
which is wired up to use the interpreter in that virtual environment so
that you don't run into that issue...

After your changes, for me, using py.test, everything except
auto/test_regularizers.py passes. Both tests in test_regularizers fail.

On 3 July 2015 at 10:48, François Chollet [email protected] wrote:

I have fixed the constraints tests. The loss weighting test succeeds when
run manually but fails on py.test; is there something about pytest I am not
getting?


Reply to this email directly or view it on GitHub
#327 (comment).


Tennessee Leeuwenburg
http://myownhat.blogspot.com/
"Don't believe everything you think"

@fchollet
Copy link
Collaborator

fchollet commented Jul 3, 2015

All tests should pass now.

fchollet pushed a commit that referenced this pull request Sep 22, 2023
* Add dtype to keras_core.operations.one_hot

* Default dtype to None and use `backend.floatx()` instead

* Use `backend.floatx()` in the __init__ itself
hubingallin pushed a commit to hubingallin/keras that referenced this pull request Sep 22, 2023
* Add dtype to keras_core.operations.one_hot

* Default dtype to None and use `backend.floatx()` instead

* Use `backend.floatx()` in the __init__ itself
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