how is this coming along @joeflack4 ?

@cmungall Thanks for pinging me. I made the updates and reversions you suggested. This should be ready to merge now, assuming the merge-check build failures I'm getting are not related to this work. Also, I left the version as 0.2.0. Feel free to change that to 0.1.0 or 0.1.1 if you like.

I changed the title of this to just be about the 'deprecated' change, since that's all that's here. With Dazhi leaving, a lot of stuff has fallen onto my plate, so I think that my progress with the MarkdownGen issue will be slow.

One other question. I had to force add the new output file Patient.md for the new Patient class we added to kitchen_sink.yaml. I see that output/ is in the .gitignore, which is why I had to force add it. Is there a reason that we are committing our test output files? Or is this a mistake and do they need to be removed?

Also, I left the version as 0.2.0. Feel free to change that to 0.1.0 or 0.1.1 if you like.

Fine to leave as is, but we need to make this clearer overall.

See #73

assuming the merge-check build failures I'm getting are not related to this work.

Hmm, can you investigate further? This is odd

One other question. I had to force add the new output file Patient.md for the new Patient class we added to kitchen_sink.yaml. I see that output/ is in the .gitignore, which is why I had to force add it. Is there a reason that we are committing our test output files? Or is this a mistake and do they need to be removed?

You are asking all the good difficult questions.

I was hoping @jiaola might have been able to document this a bit more before he left

We have this ticket #237

For this

Let's discuss this on Friday

here is the problem:

https://github.com/linkml/linkml/runs/3808748176?check_suite_focus=true#step:6:1960

kitchen_sink.yaml is used by various other tests, and this is messing up the SQL DDL test

sorry to be annoying but can you change the example and do something completely unrelated to existing classes, e.g.

  fake_class:
    deprecated: this is not a real class, we are using it to test deprecation

Hey Chris, thanks for finding the source of the issue. I made that change. Looks like the checks are failing again >,<. About to give that a check.

As for gitignore and test outputs, I added some thoughts to #237.

Edit: I looked at the test failures. There are several different errors, some of which are related to kitchen_sink.yaml, but they don't appear to be related to this particular change (at least from what I can surmise). It looks like the other PRs are failing as well.

It does look like the particular test failure you pointed out is gone though; I searched for sqlite3.OperationalError: table Address has no column named Person_id, which was [near where you linked](https://github.com/linkml/linkml/runs/3808748176?check_suite_focus=true#step:6:1960

), and I couldn't find that in the new unit test output.

OK, so this is the source of the error. It is in sqlddlgen.py:

for sqltable in self.sqlschema.tables.values():
            cols = sqltable.columns.values()
            if len(cols) == 0:
                logging.warning(f'No columns for {t.name}')
                continue

This is truly awful code by yours truly.. and so in one sense not your problem

however, the bug is only revealed when you include a class with no columns in the test schema (as I requested you did - sorry!). In fact this is an interesting test case, so I made an issue #399

But for now can you just get around this by including a fake slot in the fake class. Sorry for the runaround

You could also fix the sqlddlgen issue here (change t to sqltable) tho I slightly prefer each PR to be its own concern