search

conjure-cp / conjure

Conjure: The Automated Constraint Modelling Tool
Other
94 stars 20 forks source link

added n-fractions notebook #585

Closed N-J-Martin closed 10 months ago

N-J-Martin commented 10 months ago

Just added the n-fractions notebook and added link to the div.md page. Mostly trying this out.

ozgurakgun commented 10 months ago

Looks good! Except we probably don't want the docs/notebooks/division_n_fractions.ipynb:Zone.Identifier file, right?

ozgurakgun commented 10 months ago

also, just general git feedback. it looks like you were using the main branch of your fork, in the future it might make more sense to make a branch inside your fork so you are not working on main. not a problem for this PR though.

N-J-Martin commented 10 months ago

Looks good! Except we probably don't want the docs/notebooks/division_n_fractions.ipynb:Zone.Identifier file, right?

I'm not entirely sure what that is, I think it got added when I moved the ipynb into there. I don't know how important it is?

ozgurakgun commented 10 months ago

Quick googling: https://stackoverflow.com/questions/4496697/what-are-zone-identifier-files-and-how-do-i-prevent-them-from-being-created

I think they are probably useful when you are running the notebook but we shouldn't commit them to the repo.

ozgurakgun commented 10 months ago

Hi @N-J-Martin - if you can remove the extra file I can merge this and we can see it in action.

N-J-Martin commented 10 months ago

I removed that extra identifier file, and added another notebook and completed the L_div.md page. Can you see those updates?

ozgurakgun commented 10 months ago

I'll merge as is so we can see in the VS Code extension. The links to the notebooks aren't Colab links as far as I can see, we should fix those.